snazy commented on code in PR #282:
URL: https://github.com/apache/polaris/pull/282#discussion_r1760657185
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java:
##########
@@ -18,64 +18,77 @@
*/
package org.apache.polaris.core.storage.aws;
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonIgnore;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.base.MoreObjects;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.List;
import java.util.regex.Pattern;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
-import org.jetbrains.annotations.NotNull;
+import org.apache.polaris.immutables.PolarisImmutable;
+import org.immutables.value.Value;
+import org.immutables.value.Value.Check;
import org.jetbrains.annotations.Nullable;
/** Aws Polaris Storage Configuration information */
-public class AwsStorageConfigurationInfo extends
PolarisStorageConfigurationInfo {
+@PolarisImmutable
+@JsonSerialize(as = ImmutableAwsStorageConfigurationInfo.class)
+@JsonDeserialize(as = ImmutableAwsStorageConfigurationInfo.class)
+public abstract class AwsStorageConfigurationInfo extends
PolarisStorageConfigurationInfo {
// 5 is the approximate max allowed locations for the size of AccessPolicy
when LIST is required
// for allowed read and write locations for subscoping creds.
- @JsonIgnore private static final int MAX_ALLOWED_LOCATIONS = 5;
+ private static final int MAX_ALLOWED_LOCATIONS = 5;
// Technically, it should be
^arn:(aws|aws-cn|aws-us-gov):iam::\d{12}:role/.+$,
- @JsonIgnore public static final String ROLE_ARN_PATTERN =
"^arn:aws:iam::\\d{12}:role/.+$";
+ private static final String ROLE_ARN_PATTERN =
"^arn:aws:iam::\\d{12}:role/.+$";
- // AWS role to be assumed
- private final @NotNull String roleARN;
-
- // AWS external ID, optional
- @JsonProperty(value = "externalId", required = false)
- private @Nullable String externalId = null;
-
- /** User ARN for the service principal */
- @JsonProperty(value = "userARN", required = false)
- private @Nullable String userARN = null;
+ public static AwsStorageConfigurationInfo of(Iterable<String>
allowedLocations, String roleARN) {
+ return of(allowedLocations, roleARN, null);
+ }
- @JsonCreator
- public AwsStorageConfigurationInfo(
- @JsonProperty(value = "storageType", required = true) @NotNull
StorageType storageType,
- @JsonProperty(value = "allowedLocations", required = true) @NotNull
- List<String> allowedLocations,
- @JsonProperty(value = "roleARN", required = true) @NotNull String
roleARN) {
- this(storageType, allowedLocations, roleARN, null);
+ public static AwsStorageConfigurationInfo of(
+ Iterable<String> allowedLocations, String roleARN, @Nullable String
externalId) {
+ return ImmutableAwsStorageConfigurationInfo.builder()
+ .allowedLocations(allowedLocations)
+ .roleARN(roleARN)
+ .externalId(externalId)
+ .build();
}
- public AwsStorageConfigurationInfo(
- @NotNull StorageType storageType,
- @NotNull List<String> allowedLocations,
- @NotNull String roleARN,
- @Nullable String externalId) {
- super(storageType, allowedLocations);
- this.roleARN = roleARN;
- this.externalId = externalId;
- validateMaxAllowedLocations(MAX_ALLOWED_LOCATIONS);
+ @Override
+ public abstract List<String> getAllowedLocations();
Review Comment:
Yea - it's just a warning. But warnings are annoying.
It's okay to remove `allParameters = true`.
--
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]
