stubz151 commented on code in PR #13347:
URL: https://github.com/apache/iceberg/pull/13347#discussion_r2200933124
##########
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3InputStream.java:
##########
@@ -160,28 +187,36 @@ private void readAndCheckRanges(
.isEqualTo(Arrays.copyOfRange(original, offset, offset + length));
}
- @Test
- public void testClose() throws Exception {
+ @ParameterizedTest
+
@MethodSource("org.apache.iceberg.aws.s3.S3TestUtil#analyticsAcceleratorLibraryProperties")
+ public void testClose(Map<String, String> aalProperties) throws Exception {
+ final S3FileIOProperties s3FileIOProperties = new
S3FileIOProperties(aalProperties);
+ skipIfAnalyticsAcceleratorEnabled(
+ s3FileIOProperties,
+ "Analytics Accelerator Library has different exception handling when
closed");
S3URI uri = new S3URI("s3://bucket/path/to/closed.dat");
- SeekableInputStream closed = newInputStream(s3, uri);
+ SeekableInputStream closed = newInputStream(s3, s3Async, uri,
aalProperties);
closed.close();
assertThatThrownBy(() -> closed.seek(0))
.isInstanceOf(IllegalStateException.class)
.hasMessage("already closed");
}
- @Test
- public void testSeek() throws Exception {
- testSeek(s3);
+ @ParameterizedTest
+
@MethodSource("org.apache.iceberg.aws.s3.S3TestUtil#analyticsAcceleratorLibraryProperties")
+ public void testSeek(Map<String, String> aalProperties) throws Exception {
+ testSeek(s3, s3Async, aalProperties);
}
- protected void testSeek(S3Client s3Client) throws Exception {
+ protected void testSeek(
+ S3Client s3Client, S3AsyncClient s3AsyncClient, Map<String, String>
aalProperties)
+ throws Exception {
S3URI uri = new S3URI("s3://bucket/path/to/seek.dat");
byte[] expected = randomData(1024 * 1024);
writeS3Data(uri, expected);
- try (SeekableInputStream in = newInputStream(s3Client, uri)) {
+ try (SeekableInputStream in = newInputStream(s3Client, s3AsyncClient, uri,
aalProperties)) {
Review Comment:
Thanks for the review @geruh
I think it's necessary because AAL makes use of both for now, so we will
always sometimes have to pass both for that.
I think this way makes sense because of this, but am open to idea's here.
--
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]
