jrouly commented on code in PR #490: URL: https://github.com/apache/iceberg-python/pull/490#discussion_r1509828986
########## pyiceberg/catalog/glue.py: ########## @@ -252,6 +259,22 @@ def _construct_database_input(database_name: str, properties: Properties) -> Dat return database_input +def _register_glue_catalog_id_with_glue_client(glue: GlueClient, glue_catalog_id: str) -> None: + """ + Register the Glue Catalog ID (AWS Account ID) as a parameter on all Glue client methods. + + It's more ergonomic to do this than to pass the CatalogId as a parameter to every client call since it's an optional + parameter and boto3 does not support 'None' values for missing parameters. Review Comment: I could (very easily) be convinced that this is indeed _not_ more ergonomic, and that it makes more sense to add the actual `CatalogId` parameter to every single invocation of a Glue client call. That looks something like this: ```python def _glue_kwargs(self) -> dict[str, str]: if self.glue_catalog_id is not None: return dict(CatalogId=self.glue_catalog_id) return dict() ... # Add this kwarg expansion to every single client call. self.glue.get_databases(**self._glue_kwargs()) ``` It works, it just gets a little noisy. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org