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 :nomore_files to return stream #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

heri16
Copy link

@heri16 heri16 commented Jul 26, 2016

Currently, {:new_file, nil} is not returned after the last file, this makes it difficult for downstream to know that the last chunk has been received. This means that the downstream cannot return an item indicating that all binary chunks of the last file have been fully received/processes.

For example, if using after_fun in ZipStream.unzip |> Stream.transform, there would be no way to return, {:ok, file_name} indicating that the last file is complete.

Previously, {:new_file, name} is not returned after the last file, this makes it difficult for downstream to know that the last chunk has been received, so as to return a stream item indicating that all binary chunks of the file has been fully received/processes.
For example, if using after_fun in `ZipStream.unzip |> Stream.transform`, there would be no way to return, `{:ok, file_name}` indicating that the last file is complete.
API is changes as `:nomore_files` atom is expected in the returned stream from `ZipStream.unzip/1`
@awetzel
Copy link
Collaborator

awetzel commented Jul 28, 2016

Yes good catch, useful because the :halt of the stream can happen before the end of the stream (for instance with an Enum.take). The drawback is that the new API is not backward compatible so I/and other user, have to modify projects using this lib (that's why the version bump in the PR is welcomed :) ). But I am not sure why the tests do not pass anymore, did you investigate ?

PS: thank you !

@heri16
Copy link
Author

heri16 commented Jul 29, 2016

Ah... I missed the tests. Will work on them and get back.

On Friday, 29 July 2016, Arnaud Wetzel [email protected] wrote:

Yes good catch, useful because the :halt of the stream can happen before
the end of the stream (for instance with an Enum.take). The drawback is
that the new API is not backward compatible so I/and other user, have to
modify projects using this lib (that's why the version bump in the PR is
welcomed :) ). But I am not sure why the tests do not pass anymore, did you
investigate ?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAgK_WJ0JocRLLQpOWGaldKiHdQw5S3Tks5qaOB6gaJpZM4JVM3e
.

@heri16
Copy link
Author

heri16 commented Jul 29, 2016

Is there a style of API that you prefer? Is :nomore_files ok, or something else? @awetzel

@ananthakumaran
Copy link

The typical way is to just use Stream.concat(stream, [:__eof__])

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