You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@polaris.apache.org by "rishii-19-works (via GitHub)" <gi...@apache.org> on 2026/01/05 17:27:22 UTC

[PR] Add config to disable S3 trailing checksum validation [polaris]

rishii-19-works opened a new pull request, #3357:
URL: https://github.com/apache/polaris/pull/3357

   This PR adds a config-driven, S3-compatible way to disable AWS SDK trailing
   checksum validation by wiring the setting through Iceberg’s S3FileIO properties.
   
   - Default behavior remains unchanged
   - No provider-specific logic
   - Applies to all S3-compatible object stores
   
   Local note: Spotless passes locally. Full build fails on Windows during unrelated
   JAR packaging tasks; CI on Linux should validate.
   
   Fixes #3346
   


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add config to disable S3 trailing checksum validation [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.

dimas-b commented on code in PR #3357:
URL: https://github.com/apache/polaris/pull/3357#discussion_r2670266228


##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java:
##########
@@ -107,6 +107,13 @@ public URI getInternalEndpointUri() {
   /** Flag indicating whether path-style bucket access should be forced in S3 clients. */
   public abstract @Nullable Boolean getPathStyleAccess();
 
+  /**
+   * Flag indicating whether S3 trailing checksum validation should be enabled. Defaults to true
+   * when unset.
+   */
+  @Nullable
+  public abstract Boolean getEnableTrailingChecksums();

Review Comment:
   This seems to be a "dangling" property 🤔 Is it possible for a user to set it? It also does not appear to be set in code 🤔 (basically what @adnanhemani commented)



##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java:
##########
@@ -49,13 +49,11 @@ public FileIO loadFileIO(
       @Nonnull String ioImplClassName,
       @Nonnull Map<String, String> properties) {
 
-    // Get subcoped creds
+    // Get subscoped creds
     properties = new HashMap<>(properties);
 
     // Update the FileIO with the subscoped credentials
-    // Update with properties in case there are table-level overrides the credentials should
-    // always override table-level properties, since storage configuration will be found at
-    // whatever entity defines it
+    // Storage-level configuration should override table-level properties

Review Comment:
   These changes LGTM, but please move them into a separate PR to avoid conflating "cleanup" with feature work.



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org