[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #5041: AWS: add skip name validation config for glue namespace

[GitBox <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #5041:
URL: https://github.com/apache/iceberg/pull/5041#discussion_r897393831


##########
aws/src/main/java/org/apache/iceberg/aws/glue/IcebergToGlueConverter.java:
##########
@@ -105,31 +105,41 @@ static void validateNamespace(Namespace namespace) {
 
   /**
    * Validate and convert Iceberg namespace to Glue database name
-   * @param namespace Iceberg namespace
+   *
+   * @param namespace                Iceberg namespace
+   * @param shouldSkipNameValidation should skip name validation
    * @return database name
    */
-  static String toDatabaseName(Namespace namespace) {
-    validateNamespace(namespace);
+  static String toDatabaseName(Namespace namespace, boolean shouldSkipNameValidation) {
+    if (!shouldSkipNameValidation) {
+      validateNamespace(namespace);
+    }

Review Comment:
   newline after if block



##########
aws/src/main/java/org/apache/iceberg/aws/glue/IcebergToGlueConverter.java:
##########
@@ -105,31 +105,41 @@ static void validateNamespace(Namespace namespace) {
 
   /**
    * Validate and convert Iceberg namespace to Glue database name
-   * @param namespace Iceberg namespace
+   *
+   * @param namespace                Iceberg namespace
+   * @param shouldSkipNameValidation should skip name validation
    * @return database name
    */
-  static String toDatabaseName(Namespace namespace) {
-    validateNamespace(namespace);
+  static String toDatabaseName(Namespace namespace, boolean shouldSkipNameValidation) {

Review Comment:
   Similar to above, I think the boolean can just be skipNameValidation



##########
aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java:
##########
@@ -450,7 +453,7 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept
 
     glue.deleteDatabase(DeleteDatabaseRequest.builder()
         .catalogId(awsProperties.glueCatalogId())
-        .name(IcebergToGlueConverter.toDatabaseName(namespace))
+        .name(IcebergToGlueConverter.toDatabaseName(namespace, awsProperties.glueCatalogSkipNameValidation()))

Review Comment:
   I think skipNameValidation should suffice (I don't think we need the glueCatalog part in the name)



-- 
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@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org