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

=htc #20236 enable streaming responses with Connection: close #20254

Merged
merged 1 commit into from Apr 7, 2016

Conversation

aruediger
Copy link
Contributor

fixes #20236

/cc @sirthias

@akka-ci
Copy link

akka-ci commented Apr 7, 2016

Can one of the repo owners verify this patch?

@ktoso
Copy link
Member

ktoso commented Apr 7, 2016

OK TO TEST

@ktoso
Copy link
Member

ktoso commented Apr 7, 2016

LGTM, awesome catch - thanks a lot @2Beaucoup !

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels Apr 7, 2016
@akka-ci
Copy link

akka-ci commented Apr 7, 2016

Test PASSed.

@@ -169,21 +169,17 @@ private[http] object OutgoingConnectionBlueprint {
private def entitySubstreamStarted = entitySource ne null
private def idle = this
private var completionDeferred = false
private var completeOnMessageEnd = false
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't the existing boolean field be used for both completion scenarios? (doesn't matter much, just curious)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought about that too but wasn't sure if completionDeferred is guaranteed to be set after completeOnMessageEnd. completionDeferred |= closeRequested should work though.

@johanandren
Copy link
Member

LGTM

@ktoso ktoso merged commit 78b88c4 into akka:master Apr 7, 2016
@ktoso
Copy link
Member

ktoso commented Apr 7, 2016

Thanks again!

On 7 April 2016 at 18:39:46, Johan Andrén (notifications@github.com) wrote:

LGTM


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@aruediger aruediger deleted the connection-close-client branch May 6, 2016 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection: close breaks streaming responses
4 participants