>From Peeyush Gupta <[email protected]>: Attention is currently required from: Hussain Towaileb.
Peeyush Gupta has posted comments on this change by Hussain Towaileb. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20743?usp=email ) Change subject: [ASTERIXDB-3634][EXT]: Add support to iceberg catalogs ...................................................................... Patch Set 7: (5 comments) File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/awsclient/EnsureCloseAWSClientFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20743/comment/e4c69a42_bb223a47?usp=email : PS7, Line 39: private static final long serialVersionUID = 1L; : : private transient Map<String, String> properties; : private transient String instanceId; : : private transient volatile CloseableAwsClients s3Clients; : private transient volatile CloseableAwsClients glueClients; : : private transient volatile boolean closed; : private transient volatile boolean registered; You have used serialVersionUID and transient variables, do you need to extend Serializable interface as well? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/AwsUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20743/comment/de6c93e6_72d16169?usp=email : PS7, Line 199: appCtx != null Is this for extra precaution or there particular situations appCtx can be null here? Also, if it is null, should we not instead error out? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/aws/iceberg/glue/GlueUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20743/comment/3b8d3177_37a19d7d?usp=email : PS7, Line 106: "glue" Make this a constant. File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/google/GCSUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20743/comment/f099e965_17e5b459?usp=email : PS7, Line 202: int duration = 900; : if (appCtx != null) { the default duration should be set properly here. can appCtx be null here? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/iceberg/nessie/NessieUtils.java: PS7: This file is quite similar to RestUtils, maybe the redundant pieces of code can be reduced. This can be done in a later patch if needed. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20743?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I94f00d4d7c7d130054337696286e921e940fc6bc Gerrit-Change-Number: 20743 Gerrit-PatchSet: 7 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Peeyush Gupta <[email protected]> Gerrit-Attention: Hussain Towaileb <[email protected]> Gerrit-Comment-Date: Tue, 06 Jan 2026 22:00:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
