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.

Sign up for GitHub

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

Jump to bottom

[core] Fix LocalServer failure to handle large body request (Issue #481) #504

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[core] Fix LocalServer failure to handle large body request (Issue #481) #504

wants to merge 3 commits into from

Conversation

sebsto
Copy link
Contributor

@sebsto sebsto commented Mar 12, 2025

Fix #481

@sebsto sebsto added kind/bug Feature doesn't work as expected. 🔨 semver/patch No public API change. labels Mar 12, 2025
@sebsto sebsto added this to the 2.0 milestone Mar 12, 2025
@sebsto sebsto requested a review from fabianfett March 12, 2025 22:03
@sebsto sebsto self-assigned this Mar 12, 2025
@sebsto sebsto mentioned this pull request Mar 12, 2025
Open
fabianfett
fabianfett requested changes Mar 17, 2025
View reviewed changes
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

We should fix this by adding streaming support here.

@sebsto
Copy link
Contributor Author

sebsto commented Mar 18, 2025 •
edited
Loading

@fabianfett Let's separate concerns. This fix handles the incoming data to the local server (what the client sends us)
Let's open a separate issue and PR to fix the outgoing data, i.e. the streaming response

@sebsto sebsto requested a review from fabianfett March 19, 2025 17:38
@sebsto
Copy link
Contributor Author

sebsto commented Mar 24, 2025

@fabianfett We need to take a decision.
I understand your preference is to revert back to non NIOAsync implementation of the local server.
But while waiting for this, can we merge this one line change that will unblock #481 ?

Then we can plan for a proper revert to v1 implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabianfett fabianfett Awaiting requested review from fabianfett

Requested changes must be addressed to merge this pull request.

Assignees

@sebsto sebsto

Labels
kind/bug Feature doesn't work as expected. 🔨 semver/patch No public API change.
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

Local Debugging: LambdaHttpServer Drops All But Last Inbound Body Chunk
2 participants
@sebsto @fabianfett