-
Notifications
You must be signed in to change notification settings - Fork 28
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
misc: enhance support for replayable instances of InputStream
#1197
Conversation
This comment has been minimized.
This comment has been minimized.
InputStream
public fun InputStream.asByteStream(contentLength: Long? = null): ByteStream.SourceStream { | ||
val source = source() | ||
if (markSupported() && contentLength != null) { | ||
mark(contentLength.toInt()) | ||
} | ||
|
||
return object : ByteStream.SourceStream() { | ||
override val contentLength: Long? = contentLength | ||
override val isOneShot: Boolean = !markSupported() | ||
override fun readFrom(): SdkSource = source | ||
override fun readFrom(): SdkSource { | ||
if (markSupported() && contentLength != null) { | ||
reset() | ||
mark(contentLength.toInt()) | ||
return object : SdkSource by source() { | ||
// no-op close | ||
override fun close() { } | ||
} | ||
} | ||
|
||
return source() | ||
} | ||
} | ||
} |
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.
Question: Is this a behavioral change wrt closing? It seems like before we returned the raw underlying SdkSource
before which, if closed, would close the underlying InputStream
. Now it looks like that won't happen...why is that necessary?
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.
Yes it is. Without this change the underlying InputStream
is closed after hashing (because we call a .use { }
on the SdkSource
) and subsequent attempts to read the body (e.g. during transmission) fail with IOException: Stream closed
This comment has been minimized.
This comment has been minimized.
Affected ArtifactsChanged in size
|
Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.