MonkeyCanCode commented on code in PR #3215:
URL: https://github.com/apache/polaris/pull/3215#discussion_r2594975754


##########
site/content/in-dev/unreleased/command-line-interface.md:
##########
@@ -156,7 +158,7 @@ options:
       --consent-url  (Only for Azure) A consent URL granting permissions for 
the Azure Storage location
       --service-account  (Only for GCS) The service account to use when 
connecting to GCS
       --property  A key/value pair such as: tag=value. Multiple can be 
provided by specifying this option more than once
-      --catalog-connection-type  The type of external catalog in 
[iceberg-rest, hadoop, hive].
+      --catalog-connection-type  The type of external catalog in [ICEBERG, 
HADOOP, HIVE].

Review Comment:
   so it doesn't appear it matters:
   ```
           Argument(Arguments.CATALOG_CONNECTION_TYPE, str,
                    Hints.Catalogs.External.CATALOG_CONNECTION_TYPE, lower=True,
                    choices=[ct.value for ct in CatalogConnectionType]),
   ```
   as we are applying `lower=True` anyway. Also, in other places, we are refers 
to upper case ones as well:
   ```
   # Allows both REST and HIVE connection type
   polaris.features."SUPPORTED_CATALOG_CONNECTION_TYPES"=["ICEBERG_REST","HIVE"]
   ```
   
   Also, as all other arguments with enum are pass in as upper case, should we 
keep them as upper case them. For example:
   ```
               CATALOG_AUTHENTICATION_TYPE = (
                   "The type of authentication in [OAUTH, BEARER, SIGV4, 
IMPLICIT]"
               )
   ...
           Argument(Arguments.CATALOG_AUTHENTICATION_TYPE, str,
                    Hints.Catalogs.External.CATALOG_AUTHENTICATION_TYPE, 
lower=True,
                    choices=[at.value for at in AuthenticationType]),
   ```



-- 
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]

Reply via email to