evindj commented on code in PR #3496:
URL: https://github.com/apache/polaris/pull/3496#discussion_r2710952522
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java:
##########
@@ -124,6 +124,13 @@ public URI getStsEndpointUri() {
return getStsEndpoint() == null ? getInternalEndpointUri() :
URI.create(getStsEndpoint());
}
+ @JsonIgnore
+ public boolean isAwsS3() {
+ String endpoint = getEndpoint();
+ // AWS S3 if no endpoint is specified or if it uses an amazonaws.com
endpoint
+ return endpoint == null || endpoint.contains(".amazonaws.com");
+ }
+
@JsonIgnore
Review Comment:
It is also a question if we want a single class for all S3 compatible
storage or if we want to have a model where there are many subclasses based on
the actual storage backend we are dealing with.
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java:
##########
@@ -124,6 +124,13 @@ public URI getStsEndpointUri() {
return getStsEndpoint() == null ? getInternalEndpointUri() :
URI.create(getStsEndpoint());
}
+ @JsonIgnore
+ public boolean isAwsS3() {
+ String endpoint = getEndpoint();
+ // AWS S3 if no endpoint is specified or if it uses an amazonaws.com
endpoint
+ return endpoint == null || endpoint.contains(".amazonaws.com");
Review Comment:
can the endpoint be an Ip address rather than than a FQDN ?
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java:
##########
@@ -124,6 +124,13 @@ public URI getStsEndpointUri() {
return getStsEndpoint() == null ? getInternalEndpointUri() :
URI.create(getStsEndpoint());
}
+ @JsonIgnore
+ public boolean isAwsS3() {
+ String endpoint = getEndpoint();
+ // AWS S3 if no endpoint is specified or if it uses an amazonaws.com
endpoint
+ return endpoint == null || endpoint.contains(".amazonaws.com");
+ }
+
@JsonIgnore
Review Comment:
Probably not this change but is weird that the class is called
AwsStorageConfigurationInfo and it implements a method with this name, I think
eventually we want to refactor this class and may be call it
```S3StorageConfigurationInfo``` instead.
--
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]
