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 ZipToStreamFlow #170

Closed
wants to merge 4 commits into from
Closed

add ZipToStreamFlow #170

wants to merge 4 commits into from

Conversation

kirked
Copy link

@kirked kirked commented Jul 25, 2019

Adds a facility for predictable-memory streaming zipfile creation, as in the instance of sending multiple files to a browser over an HTTP connection (even when they originate from a remote service such as S3).

Incoming InputStreams must be able to be created synchronously, but are delayed such that only a single InputStream is open at any given time.

Original gist at https://gist.github.com/kirked/03c7f111de0e9a1f74377bf95d3f0f60 (original license MIT).

The author is happy to contribute this and derivative works for inclusion in the akka-stream-contrib library, and hereby assigns full control over this code, including copyright protection and licensing worldwide, in perpetuity, to Lightbend, Inc.

Purpose

References

References #xxxx

Changes

Background Context

Doug Kirk added 3 commits July 25, 2019 10:49
Adds a facility for predictable-memory streaming zipfile creation,
as in the instance of sending multiple files to a browser over an
HTTP connection (even when they originate from a remote service
such as S3).

Incoming streams must be able to be created synchronously, but are
delayed such that only a single InputStream is open at any given time.

Original gist at https://gist.github.com/kirked/03c7f111de0e9a1f74377bf95d3f0f60
(original license MIT).

The author is happy to contribute this and derivative works for
inclusion in the akka-stream-contrib library, and hereby assigns
full control over this code, including copyright protection and
licensing worldwide, in perpetuity, to Lightbend, Inc.
Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Cool

}

object ZipToStreamFlow {
final case class ZipSource(filePath: String, streamGenerator: () => InputStream)
Copy link
Member

Choose a reason for hiding this comment

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

It might be nicer to use Akka streams in this API (even if they'd eventually need to be converted to InputStream - WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. When I wrote this, our need was for zipping multiple S3-contained assets to the browser, and the AWS API (at least that we were using) provided synchronous access to the InputStreams. There was no Alpakka, and Akka was just joining our tech stack (I still have the original Play Iteratee version as a gist).

I see this as a good start for an often-needed utility that can be greatly improved.

Copy link
Member

Choose a reason for hiding this comment

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

I also prefer the implementation to use Akka Streams API. Convertion from Java stream to Akka Stream is always possible with StreamConverters: https://github.com/akka/akka/blob/master/akka-stream/src/main/scala/akka/stream/scaladsl/StreamConverters.scala#L25

@2m
Copy link
Member

2m commented Aug 14, 2019

This is useful, but as discussed, it would be better to implement this functionality using Akka Stream APIs.

I am going to close this PR here, but the way forward for Zip streams would be to move the implementation of #171 to Alpakka File connector and then enrich that with some examples of Java Streams and using akka.stream.scaladsl.StreamConverters. That should cover the same use-cases as are solved in this PR.

@kirked feel free to move the #171 to Alpakka and work together with @michalbogacz on this.

@2m 2m closed this Aug 14, 2019
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.

None yet

3 participants