iceberg-python
iceberg-python copied to clipboard
Glue endpoint config variable
Closes: https://github.com/apache/iceberg-python/issues/414
I don't know java, but tried to stay true to the implementation, at least in regards to the commenting.
Open Questions:
- I did a very basic test, but I wasn't sure if I should make the test more robust in any regard. Let me know if you think of an edge case I am missing. I could test creating/listing a namespace if that would be helpful to confirm its truly working properly, but that felt like a moto issue, rather than something that should be tested within pyiceberg.
- On my end, I am using
glue.endpoint
for all pytests. I was not sure if within pyiceberg pytests, we wanted to migrate all tests to the endpoint, or continue to usemock_aws()
. I started with the least invasive option, but can migrate all other glue tests if that is helpful. - I am not seeing a ton of docs regarding glue config variables, and its is undocumented in the main docs. Within pyiceberg docs, I do see this Glue config, but not sure if this is the best place to put it. Should this get added to the docs, or should it remain undocumented, since its not a commonly used feature?
@Fokko @HonahX both of your thoughts or reviews would be helpful, thanks!
On my end, I am using glue.endpoint for all pytests. I was not sure if within pyiceberg pytests, we wanted to migrate all tests to the endpoint, or continue to use mock_aws(). I started with the least invasive option, but can migrate all other glue tests if that is helpful.
I'm not a big fan of mocking, but I'll defer this question to @HonahX since he's the main author.
I am not seeing a ton of docs regarding glue config variables, and its is undocumented in the main docs. Within pyiceberg docs, I do see this Glue config, but not sure if this is the best place to put it. Should this get added to the docs, or should it remain undocumented, since its not a commonly used feature?
Not documenting features is a mistake, we should document everything as much as possible, without losing the end-user :)
@sebpretzer FYI, I just found an error in the current glue integration test: #536 . Please feel free to include this fix in this PR if you want to add glue.endpoint
to the integration tests
Hi @sebpretzer. Gently checking in to see if you have time to update this PR. If not, I’d be happy to help and take over the changes. It will be great to include this in 0.7.0
Close in favor of https://github.com/apache/iceberg-python/pull/920