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

Option for immediate header flush #575

Merged
merged 2 commits into from
May 28, 2019

Conversation

Bobochka
Copy link
Contributor

In case of body stream if first body bytes generation takes long this adds an option of "faster response".

Will add tests if general idea is ok.

@erikdubbelboer
Copy link
Collaborator

I'm wondering if this should be two functions of if it should just be an exported field named Response.FlushHeaders bool. What do you think @kirillDanshin ?

@Bobochka
Copy link
Contributor Author

@erikdubbelboer @kirillDanshin Exported field and added tests

Copy link
Collaborator

@kirillDanshin kirillDanshin left a comment

Choose a reason for hiding this comment

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

LGTM, but needs some production testing before merge, I think

@Bobochka
Copy link
Contributor Author

@kirillDanshin any action from my side? We're already running a fork in prod that always flushes before body.

@erikdubbelboer erikdubbelboer merged commit f14dea7 into valyala:master May 28, 2019
@erikdubbelboer
Copy link
Collaborator

Thanks. We don't need any more testing seeing as this doesn't change anything if you don't set the value to true. And even then, just calling w.Flush() is not something complicated.

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.

3 participants