singhpk234 commented on a change in pull request #4280:
URL: https://github.com/apache/iceberg/pull/4280#discussion_r830249198
##########
File path: aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java
##########
@@ -299,6 +332,9 @@ public AwsProperties(Map<String, String> properties) {
this.glueCatalogId = properties.get(GLUE_CATALOG_ID);
this.glueCatalogSkipArchive = PropertyUtil.propertyAsBoolean(properties,
AwsProperties.GLUE_CATALOG_SKIP_ARCHIVE,
AwsProperties.GLUE_CATALOG_SKIP_ARCHIVE_DEFAULT);
+ this.glueLakeFormationEnabled = PropertyUtil.propertyAsBoolean(properties,
Review comment:
[minor] I think we should also initialize the default value of
glueLakeFormationEnabled
in the constructor above #L319
##########
File path: aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java
##########
@@ -389,6 +425,14 @@ public void setGlueCatalogSkipArchive(boolean skipArchive)
{
this.glueCatalogSkipArchive = skipArchive;
}
+ public boolean glueLakeFormationEnabled() {
+ return this.glueLakeFormationEnabled;
Review comment:
[nit] can just do glueLakeFormationEnabled;
like the one's done in other getter's
##########
File path: aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java
##########
@@ -250,6 +264,24 @@
@Deprecated
public static final boolean CLIENT_ENABLE_ETAG_CHECK_DEFAULT = false;
+ /**
+ * Used by {@link LakeFormationAwsClientFactory}.
+ * The table name used as part of lake formation credentials request.
+ */
+ public static final String LAKE_FORMATION_TABLE_NAME =
"lakeformation.table-name";
+
+ /**
+ * Used by {@link LakeFormationAwsClientFactory}.
+ * The database name used as part of lake formation credentials request.
+ */
+ public static final String LAKE_FORMATION_DB_NAME = "lakeformation.db-name";
+
+ /**
+ * Used by {@link LakeFormationAwsClientFactory}.
+ * The account ID used as part of lake formation credentials request.
+ */
+ public static final String LAKE_FORMATION_ACCOUNT_ID =
"lakeformation.account-id";
Review comment:
[question] Thinking out loud here :
what if the end-user forget's to add any of these conf's, he will see AWS
stacktrace that the ARN is invalid, could we in principle fail him earlier with
IllegalArg / Validation Error, as we do by checking the awsClientFactory is
instance of LF ? WDYT
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
