laserninja commented on code in PR #16112:
URL: https://github.com/apache/iceberg/pull/16112#discussion_r3165355526
##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java:
##########
@@ -469,7 +469,7 @@ private void refreshStorageCredentials() {
.collect(Collectors.toList());
if (!refreshed.isEmpty() && !isResourceClosed.get()) {
- this.storageCredentials = Lists.newArrayList(refreshed);
+ this.storageCredentials = ImmutableList.copyOf(refreshed);
Review Comment:
true, the same Kryo round-trip that fails for 'ImmutableList.of()' will also
fail for 'ImmutableList.copyOf(refreshed)' after a refresh, so the original
Kryo concern still applies on the refresh path.
I'll switch the refresh assignment back to 'Lists.newArrayList(refreshed)'
and add a Kryo round-trip test that initializes credentials, triggers a
refresh, then serializes/deserializes the FileIO via Kryo to lock in the
contract.
Since the field is private and only replaced atomically via the volatile
reference, external mutation isn't really possible, so once we revert this, the
PR effectively becomes a no-op. Happy to close it as "no change needed" if you
prefer that over keeping just the test.
--
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]
