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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
