Boto Glue standard retry policy with configuration #1307
Conversation
configurable max retry
Fokko
left a comment
There was a problem hiding this comment.
Thanks @mark-major for raising this PR! I left some suggestions, LMKWYT!
pyiceberg/catalog/glue.py
Outdated
| "max_attempts": get_first_property_value(properties, GLUE_MAX_RETRIES) | ||
| if get_first_property_value(properties, GLUE_MAX_RETRIES) | ||
| else MAX_RETRIES, | ||
| "mode": "standard", |
There was a problem hiding this comment.
Should we also make the mode configurable? :)
There was a problem hiding this comment.
I would rather not have that as an option. Standard mode is a more mature implementation compared to the legacy one.Adaptive mode is still considered 'experimental' which is not something I'd use in production.
There was a problem hiding this comment.
+1 to making it configurable just to future-proof.
Still set standard as the default, but can be overridden if necessary
There was a problem hiding this comment.
Ok, I'll make the mode configurable
There was a problem hiding this comment.
@mark-major We're almost there, are you interested in pushing this over the finish line?
There was a problem hiding this comment.
I have pushed the changes, sorry for taking this long...
Co-authored-by: Fokko Driesprong <fokko@apache.org>
Co-authored-by: Fokko Driesprong <fokko@apache.org>
kevinjqliu
left a comment
There was a problem hiding this comment.
A few nit comments. Curious about the general direction we're taking for configuring the retry mechanism for catalogs. I'm leaning towards a standardized retry config for all catalogs, similar to the commit.retry table configs https://iceberg.apache.org/docs/1.5.2/configuration/#table-behavior-properties
Curious to hear others' thoughts
| | 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 | |
There was a problem hiding this 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
Outdated
| "max_attempts": get_first_property_value(properties, GLUE_MAX_RETRIES) | ||
| if get_first_property_value(properties, GLUE_MAX_RETRIES) | ||
| else MAX_RETRIES, | ||
| "mode": "standard", |
There was a problem hiding this comment.
+1 to making it configurable just to future-proof.
Still set standard as the default, but can be overridden if necessary
Fokko
left a comment
There was a problem hiding this comment.
Thanks for following up on this 👍
* boto glue standard retry policy configurable max retry * Update configuration.md Co-authored-by: Fokko Driesprong <fokko@apache.org> * Update glue.py Co-authored-by: Fokko Driesprong <fokko@apache.org> * boto glue retry mode configurable --------- Co-authored-by: Fokko Driesprong <fokko@apache.org>
* boto glue standard retry policy configurable max retry * Update configuration.md Co-authored-by: Fokko Driesprong <fokko@apache.org> * Update glue.py Co-authored-by: Fokko Driesprong <fokko@apache.org> * boto glue retry mode configurable --------- Co-authored-by: Fokko Driesprong <fokko@apache.org>
* boto glue standard retry policy configurable max retry * Update configuration.md Co-authored-by: Fokko Driesprong <fokko@apache.org> * Update glue.py Co-authored-by: Fokko Driesprong <fokko@apache.org> * boto glue retry mode configurable --------- Co-authored-by: Fokko Driesprong <fokko@apache.org>
* boto glue standard retry policy configurable max retry * Update configuration.md Co-authored-by: Fokko Driesprong <fokko@apache.org> * Update glue.py Co-authored-by: Fokko Driesprong <fokko@apache.org> * boto glue retry mode configurable --------- Co-authored-by: Fokko Driesprong <fokko@apache.org>
Configurable retry mechanism for the Glue catalog Boto client.
standardglue.max-retrieswith default value set to 10Closes #1294