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

doc: Note that zlib.flush acts after pending writes #6172

Closed

Conversation

addaleax
Copy link
Member

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Describe that zlib.flush() may wait for pending writes. This was discussed a bit in #3782; and although it’s not really related to the original issue report there, the addition probably gives off the right impression, i.e. that flush() is not as immediate as it may sound.

@mscdex mscdex added doc Issues and PRs related to the documentations. zlib Issues and PRs related to the zlib subsystem. labels Apr 12, 2016
@jasnell
Copy link
Member

jasnell commented Apr 13, 2016

LGTM

@addaleax
Copy link
Member Author

@jasnell I’d like to land this if your lgtm meant that this resolves #3782 for you :)

@jasnell
Copy link
Member

jasnell commented Apr 18, 2016

@addaleax ... I think it partially does at least. The situation that I describe in #3782 still isn't entirely clear with this addition. An example included in the doc could likely help also but that can be added in a separate PR.

@addaleax
Copy link
Member Author

@jasnell Nah, I’m happy to add stuff to this PR.
Is there anything from #3782 (comment) you’d like to have included? I mean one could explicitly write something along the lines of .flush() respecting the highWaterMark, but that would seem to say “This stream is like all other readable streams”.

@addaleax
Copy link
Member Author

Also, 👍 on that an example for .flush() would be nice. I guess I’ll get to that tomorrow or so.

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.
@addaleax addaleax force-pushed the doc-zlib-flush-after-pending-writes branch from 61615ca to 5a84875 Compare April 18, 2016 17:00
@addaleax
Copy link
Member Author

Updated this with a bit more explanation that’s specific to the situation in #3782.

@jasnell
Copy link
Member

jasnell commented Apr 18, 2016

@addaleax +1 .. that change works for me! if there's a concise example that can illustrate the point, then awesome. If not, I'm good with this landing as is.

@addaleax
Copy link
Member Author

I’ll sleep over that, but maybe it’s just best noted as a comment in a generic .flush() example.

@stevemao
Copy link
Contributor

@nodejs/documentation

@eljefedelrodeodeljefe
Copy link
Contributor

Example would be nice if you get one together. In any case LGTM.

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.
@addaleax
Copy link
Member Author

@eljefedelrodeodeljefe Was literally finishing a first take at an example when you wrote that 😄 Would love to hear what you think of it!

@eljefedelrodeodeljefe
Copy link
Contributor

Thought so. Being in the same timezone leads to concurrent working apparently. :) I am a little foreign to zlib code, but I think that illustrates it very well. 👍

@addaleax
Copy link
Member Author

@eljefedelrodeodeljefe Keep in mind that people who are foreign to the specifics are precisely the target groups of the docs, so any questions or suggestions are greatly appreciated! :)

@addaleax
Copy link
Member Author

@jasnell Any thoughts on the second commit/the example? Are you fine with having this merged + marked as a fix for #3782? :)

@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

I like it. Thank you very much. LGTM

addaleax added a commit that referenced this pull request Apr 20, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
addaleax added a commit that referenced this pull request Apr 20, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
@addaleax
Copy link
Member Author

Landed in 5f0fcd6 and 9c2b8ec. :-)

@addaleax addaleax closed this Apr 20, 2016
@addaleax addaleax deleted the doc-zlib-flush-after-pending-writes branch April 20, 2016 16:32
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
This was referenced Apr 21, 2016
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: nodejs#3782
PR-URL: nodejs#6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: nodejs#6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 17, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 17, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstädt <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants