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 failing federated S3 tests (GSI-1148) #139

Merged
merged 5 commits into from
Nov 15, 2024
Merged

Conversation

Cito
Copy link
Member

@Cito Cito commented Nov 15, 2024

The test_federated_s3.test_populate_dummy_items sporadically failed when running on GitHub.

After spending several hours debugging, we finally found the root cause: Docker maps containers to random ports, which can be different for the IPv4 and IPv6 addresses of localhost. It can happen that the port assigned for IPv6 for one container overshadows the port assigned for IPv4 of the other container. In the federated S3 test, all S3 operations would then execute on that same container, which causes the tests to fail.

As a simple solution, this PR changes the endpoint_url from localhost to 127.0.0.1 in order to make sure that only the IPv4 port is used when accessing the localstack container.

The PR also cleans up the passing of parameters to the localstack container. The name parameter is not used and not passed any more. Also, the image used for the localstack test container has been updated from 3.5.0 to 3.8.1.

Additionally, the PR also improves the _does_object_exist method so that it properly raises possible errors (this works similarly to _does_bucket_exist now).

@Cito Cito requested a review from mephenor November 15, 2024 12:07
@coveralls
Copy link

Pull Request Test Coverage Report for Build 11855884788

Details

  • 11 of 13 (84.62%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 92.252%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/hexkit/providers/s3/provider.py 6 8 75.0%
Totals Coverage Status
Change from base Build 11853144490: 0.3%
Covered Lines: 2048
Relevant Lines: 2220

💛 - Coveralls

@Cito Cito merged commit 9cd407f into main Nov 15, 2024
8 checks passed
@Cito Cito deleted the fix/failing-federated-s3-tests branch November 15, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants