Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

use rolo twisted gateway integration - #10428

Merged
thrau merged 4 commits into
masterfrom
rolo-twisted
Mar 13, 2024
Merged

use rolo twisted gateway integration#10428
thrau merged 4 commits into
masterfrom
rolo-twisted

Conversation

@thrau

@thrau thrau commented Mar 11, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

The coded added for twisted integration in #9834 moved to rolo in localstack/rolo#8, which also includes websocket support. This PR refactors our existing code to use the new rolo version.

Changes

  • Remove duplicate code from rolo code migration
  • Use rolo 0.4.0
  • localstack.http.asgi.ASGIWebSocket is removed from the API
  • added httpx as test library to test http2 support directly

TODO

What's left to do:

  • release rolo 0.4.0
  • upgrade rolo here
  • set default gateway back to hypercorn

@thrau thrau added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Mar 11, 2024
@github-actions

github-actions Bot commented Mar 11, 2024 •
edited
Loading

Copy link
Copy Markdown

S3 Image Test Results (AMD64 / ARM64)

  2 files  ±0    2 suites  ±0   3m 18s ⏱️ +2s
393 tests ±0  342 ✅ ±0   51 💤 ±0  0 ❌ ±0 
786 runs  ±0  684 ✅ ±0  102 💤 ±0  0 ❌ ±0 

Results for commit 6034715. ± Comparison against base commit 5e21daa.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Mar 12, 2024 •
edited
Loading

Copy link
Copy Markdown

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 32m 44s ⏱️ +48s
2 707 tests +2  2 451 ✅ +2  256 💤 ±0  0 ❌ ±0 
2 709 runs  +2  2 451 ✅ +2  258 💤 ±0  0 ❌ ±0 

Results for commit 6034715. ± Comparison against base commit 5e21daa.

♻️ This comment has been updated with latest results.

alexrashed added a commit that referenced this pull request Mar 12, 2024
@alexrashed
add upper limit for rolo
c7ef8c4 
rolo==0.4.0 contains some breaking changes addressed in
#10428.
This commit pins rolo to versions lower than 0.4.0.
This limit should be removed (or at least increased) in #10428.
@alexrashed

alexrashed commented Mar 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

FYI: c7ef8c4 adds an upper limit to rolo to avoid the breaking changes, which means that this PR needs to be rebased on master after #10435, the limit has to be increased or removed, and the pins should maybe be re-generated.

Edit: Thanks @thrau for avoiding the pin at all in #10435.

@thrau
thrau marked this pull request as ready for review March 13, 2024 13:04
@thrau
thrau requested a review from bentsku March 13, 2024 13:04
@thrau

thrau commented Mar 13, 2024

Copy link
Copy Markdown
Contributor Author

the test run with twisted was successful, community against pro tests were failing due to #10444

bentsku
bentsku approved these changes Mar 13, 2024
View reviewed changes

@bentsku bentsku left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I can see the line regarding the removal of localstack.http.asgi.ASGIWebSocket has been done in #10435 instead.

Nice cleanup 🧹 awesome to see things evolving!

Comment thread localstack/aws/serving/twisted.py

@bentsku bentsku Mar 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice cleanup 👍

@thrau
thrau merged commit a6d6a3c into master Mar 13, 2024
@thrau
thrau deleted the rolo-twisted branch March 13, 2024 20:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@bentsku bentsku bentsku approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thrau @alexrashed @bentsku