singhpk234 commented on code in PR #14329:
URL: https://github.com/apache/iceberg/pull/14329#discussion_r2429735238
##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3InputFile.java:
##########
@@ -76,7 +76,7 @@ public SeekableInputStream newStream() {
if (s3FileIOProperties().isS3AnalyticsAcceleratorEnabled()) {
return AnalyticsAcceleratorUtil.newStream(this);
}
- return new S3InputStream(client(), uri(), s3FileIOProperties(), metrics());
+ return new S3InputStream(client(), uri(), s3FileIOProperties(), metrics(),
getLength());
Review Comment:
Re: getLength()
> While this does not appear to have any impact as such, it does add a lot
of noise to the logs
if this is just for logs there can be cases where the getLength() could be a
getObjectMetadata call to s3 is it a valid tradeoff ?
##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3InputStream.java:
##########
@@ -86,15 +92,20 @@ class S3InputStream extends SeekableInputStream implements
RangeReadable {
.withMaxRetries(3)
.build();
- S3InputStream(S3Client s3, S3URI location) {
- this(s3, location, new S3FileIOProperties(), MetricsContext.nullMetrics());
+ S3InputStream(S3Client s3, S3URI location, long contentLength) {
Review Comment:
can you make a new constructor for using these arguments ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
