kevinjqliu commented on code in PR #1307: URL: https://github.com/apache/iceberg-python/pull/1307#discussion_r1835489821
########## pyiceberg/catalog/__init__.py: ########## @@ -94,6 +94,7 @@ URI = "uri" LOCATION = "location" EXTERNAL_TABLE = "EXTERNAL_TABLE" +MAX_RETRIES = 10 Review Comment: nit, use the `<>_DEFAULT` convention https://github.com/apache/iceberg-python/blob/cd21f9706cd4cdc7ee9427b603b3949b005ecda7/pyiceberg/catalog/glue.py#L374 Also not sure if this property belongs in the `__init__` file, unless we're saying that this is the default for all catalogs ########## mkdocs/docs/configuration.md: ########## @@ -331,16 +331,17 @@ catalog: <!-- markdown-link-check-disable --> -| Key | Example | Description | -| ---------------------- | ------------------------------------ | ------------------------------------------------------------------------------- | -| glue.id | 111111111111 | Configure the 12-digit ID of the Glue Catalog | -| glue.skip-archive | true | Configure whether to skip the archival of older table versions. Default to true | +| Key | Example | Description | +|------------------------|----------------------------------------|---------------------------------------------------------------------------------| +| glue.id | 111111111111 | Configure the 12-digit ID of the Glue Catalog | +| glue.skip-archive | true | Configure whether to skip the archival of older table versions. Default to true | | glue.endpoint | <https://glue.us-east-1.amazonaws.com> | Configure an alternative endpoint of the Glue service for GlueCatalog to access | -| glue.profile-name | default | Configure the static profile used to access the Glue Catalog | -| glue.region | us-east-1 | Set the region of the Glue Catalog | -| glue.access-key-id | admin | Configure the static access key id used to access the Glue Catalog | -| glue.secret-access-key | password | Configure the static secret access key used to access the Glue Catalog | -| glue.session-token | AQoDYXdzEJr... | Configure the static session token used to access the Glue Catalog | +| glue.profile-name | default | Configure the static profile used to access the Glue Catalog | +| glue.region | us-east-1 | Set the region of the Glue Catalog | +| glue.access-key-id | admin | Configure the static access key id used to access the Glue Catalog | +| glue.secret-access-key | password | Configure the static secret access key used to access the Glue Catalog | +| glue.session-token | AQoDYXdzEJr... | Configure the static session token used to access the Glue Catalog | +| glue.max-retries | 10 | Configure the maximum number of retries for the Glue service calls | Review Comment: nit, should we add a retry property for each catalog? Or just use the default value in the table retry property?` ``` commit.retry.num-retries | 4 | Number of times to retry a commit before failing ``` https://iceberg.apache.org/docs/1.5.2/configuration/#table-behavior-properties ########## pyiceberg/catalog/glue.py: ########## @@ -305,7 +308,18 @@ def __init__(self, name: str, **properties: Any): aws_secret_access_key=get_first_property_value(properties, GLUE_SECRET_ACCESS_KEY, AWS_SECRET_ACCESS_KEY), aws_session_token=get_first_property_value(properties, GLUE_SESSION_TOKEN, AWS_SESSION_TOKEN), ) - self.glue: GlueClient = session.client("glue", endpoint_url=properties.get(GLUE_CATALOG_ENDPOINT)) + self.glue: GlueClient = session.client( + "glue", + endpoint_url=properties.get(GLUE_CATALOG_ENDPOINT), + config=Config( + retries={ + "max_attempts": get_first_property_value(properties, GLUE_MAX_RETRIES) + if get_first_property_value(properties, GLUE_MAX_RETRIES) + else MAX_RETRIES, + "mode": "standard", Review Comment: +1 to making it configurable just to future-proof. Still set `standard` as the default, but can be overridden if necessary -- 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
