-
Notifications
You must be signed in to change notification settings - Fork 31
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
Remove usages of Stream #74
Conversation
Hmm |
eb48808
to
c2ce315
Compare
private def readStream(stream: InputStream): ByteString = { | ||
val reader = new BufferedInputStream(stream) | ||
try ByteString(Stream.continually(reader.read).takeWhile(_ != -1).map(_.toByte).toArray) | ||
try ByteString(Iterator.continually(reader.read).takeWhile(_ != -1).map(_.toByte).toArray) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I realized what the underlying problem was, it turns out that specifically for Scala 2.12, ByteString
is missing the apply method for an Iterator
where as this exists in Scala 2.13. This is likely an oversight and a PR at Pekk was created at apache/pekko#257 to resolve this.
When that PR lands its possible to remove the .toArray
, in any case both variants are equivalent to eachother because internally in the ByteString.apply
it converts it to an Array
anyways.
@pjfanning PR is now ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Completely removes usages of
Stream
, making #71 redudnant.Resolves: #71