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 broken aws tests #2658

Merged
merged 2 commits into from Feb 14, 2019
Merged

Fix broken aws tests #2658

merged 2 commits into from Feb 14, 2019

Conversation

dlstadther
Copy link
Collaborator

Description

Update botocore minimum version to align with s3transfer=0.2.0 to avoid ImportError: cannot import name ReadTimeoutError

Motivation and Context

Master tests are failing for AWS

Have you tested this? If so, how?

Travis

@dlstadther
Copy link
Collaborator Author

Current patch removes ReadTimeoutError, but introduces an issue with moto actually mocking boto3 services.

Altered local solution to lock s3transfer==0.1.13 and tests pass. Need to find out why s3transfer==0.2.0 cause these issues.

@dlstadther
Copy link
Collaborator Author

Not saying we have to lock s3transfer version, but want to verify while i return to other tasks.

@honnix
Copy link
Contributor

honnix commented Feb 13, 2019

So my interpretation of this PR is still WIP?

@dlstadther
Copy link
Collaborator Author

@honnix This PR fixes the failed tests, but it locks us into the previous version of s3transfer. I looked through its commits between version 0.1.13 and 0.2.0 and didn't immediately see anything major.

I'm not convinced this is the best solution. But it does show that something about s3transfer==0.2.0 causes our moto tests to break

@TMaYaD
Copy link
Contributor

TMaYaD commented Feb 13, 2019

@dlstadther Should I include this in my PRs to make sure tests pass? FWIW, the PRs have nothing to do with AWS directly.

@dlstadther
Copy link
Collaborator Author

dlstadther commented Feb 13, 2019

@TMaYaD I would not include this yet. I'd like to get a better understanding of why this fixes the tests and provide a more sustainable solution.

If your PR(s) don't touch AWS stuff and those 2 are the only failing test suites, there's no reason to prevent that merge.

I'll try to take a look at your PRs, but work is a bit busy at the moment!

@Tarrasch Tarrasch merged commit 07007c7 into spotify:master Feb 14, 2019
@Tarrasch
Copy link
Contributor

I merged as I think being green is important - I hope you guys don't mind!

@dlstadther dlstadther deleted the hotfix/tox-aws branch December 8, 2020 22:39
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.

None yet

4 participants