Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add http2-server to aws-replicator #64

Merged
merged 2 commits into from
Jun 6, 2024
Merged

add http2-server to aws-replicator #64

merged 2 commits into from
Jun 6, 2024

Conversation

thrau
Copy link
Member

@thrau thrau commented Jun 5, 2024

This is a provisional measure so we can move forward with removing quart and the leftover http2 server code from localstack.

I tried for an afternoon to replace the http2 server completely with a new gateway/handler chain approach in #62 , but just couldn't get it working. I got impatient so I decided to just move the http2_server.py from localstack to here, so we can remove it upstream.

@thrau thrau requested a review from alexrashed June 5, 2024 20:46
@thrau thrau requested a review from whummer as a code owner June 5, 2024 20:46
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks a lot for moving this code here as an intermediary step! This allows us to move forward with getting rid of the dependencies in localstack-core.
I only added a comment to add hypercorn to the dependencies of this extension to prepare its removal in localstack-core as well.

Comment on lines +13 to +17
from hypercorn import utils as hypercorn_utils
from hypercorn.asyncio import serve, tcp_server
from hypercorn.config import Config
from hypercorn.events import Closed
from hypercorn.protocol import http_stream
Copy link
Member

Choose a reason for hiding this comment

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

Would be great to add hypercorn to the list of dependencies of this extension.
While quart is explicitly set in the dependencies of this extension, hypercorn and its transitive dependency h11 are not. This is not yet a problem for removing quart from the list of dependencies in localstack-core, but since hypercorn is not the default gateway implementation anymore it is likely to be removed sometime soon as well.

@thrau thrau merged commit b6b129a into main Jun 6, 2024
1 check passed
@thrau thrau deleted the inline-http2-server branch June 6, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants