Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update S3ParquetTestBase testReadWriteUsingProfile #6310

Merged

Conversation

devinrsmith
Copy link
Member

@devinrsmith devinrsmith commented Oct 29, 2024

This fixes an nightly error that #6305 introduced where S3ParquetLocalStackTest.testReadWriteUsingProfile was not erroring out with the use of an invalid profile or credentials data (as far as the earlier images were concerned, localstack was failing based on the region and MinIO was failing based on the keys). While we might want to have some sort of test like this, it should be standalone and not tied to the usage of profiles. The invalid keys based test was separated out and only enabled for MinIO. An "invalid" region does not trigger this error for MinIO nor Localstack.

Ultimately, the behavior was traced down to a difference in behavior between localstack 3.6 and localstack 3.7; the logic must have changed in some way. We could do a deep dive into the commits to figure out why the behavior changed, or file an issue, if we care to pursue this further.

https://github.com/localstack/localstack/releases/tag/v3.7.0

This fixes an nightly error that deephaven#6305 introduced where `S3ParquetLocalStackTest.testReadWriteUsingProfile` was not erroring out with the use of an "invalid" region name. While we might want to have some sort of test like this, it should be standalone and not tied to the usage of profiles. As such, the corresponding code was removed.

Ultimately, the behavior was traced down to a difference in behavior between localstack 3.6 and localstack 3.7; the region name logic must have changed in some way. We could do a deep dive into the commits to figure out why the behavior changed, or file an issue, if we care to pursue this further.

https://github.com/localstack/localstack/releases/tag/v3.7.0
@devinrsmith devinrsmith added this to the 0.37.0 milestone Oct 29, 2024
@devinrsmith devinrsmith self-assigned this Oct 29, 2024
@devinrsmith devinrsmith merged commit 56e3cdf into deephaven:main Oct 29, 2024
18 checks passed
@devinrsmith devinrsmith deleted the nightly/fix-localstack-failure branch October 29, 2024 22:30
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants