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

Update awscli, botocore, boto3 and moto in Travis #2627

Closed
wants to merge 40 commits into from

Conversation

nikochiko
Copy link
Contributor

Reference: #2604
Solve the issue of botocore requirement getting overwritten during travis.

Changes proposed:

  • Update awscli, botocore, boto3 and moto to their latest versions

@nikochiko nikochiko changed the title Update awscli, botocore. boto3 and moto Update awscli, botocore, boto3 and moto Jan 18, 2020
@nikochiko nikochiko changed the title Update awscli, botocore, boto3 and moto Update awscli, botocore, boto3 and moto in Travis Jan 18, 2020
@nikochiko

This comment has been minimized.

@codecov-io
Copy link

codecov-io commented Jan 18, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@3f36692). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #2627   +/-   ##
=========================================
  Coverage          ?   73.21%           
=========================================
  Files             ?       82           
  Lines             ?     5338           
  Branches          ?        0           
=========================================
  Hits              ?     3908           
  Misses            ?     1430           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f36692...e43ae8c. Read the comment docs.

@nikochiko
Copy link
Contributor Author

Second update:

Only mention awscli in .travis.yml and install awscli's version of botocore

Results:

Build is passing. I added some tests for aws_utils to confirm that the versions are compatible with each other.

Notes:

botocore requirement got overwritten by awscli.
Travis throws this warning:

boto3 1.9.88 has requirement botocore<1.13.0,>=1.12.88, but you'll have botocore 1.14.5 which is incompatible.
boto3 1.9.88 has requirement s3transfer<0.2.0,>=0.1.10, but you'll have s3transfer 0.3.1 which is incompatible.
aws-sam-translator 1.20.1 has requirement jsonschema~=3.0, but you'll have jsonschema 2.6.0 which is incompatible.

@yashdusing
Copy link
Contributor

@nikochiko instead of doing this. Could you try installing awscli and coveralls first.
Then install requirements/dev.txt
and in requirements/dev.txt remove boto3, botocore, moto versions

@nikochiko
Copy link
Contributor Author

@yashdusing Yes! Doing the same.
Directly making the changes from stackoverflow (https://stackoverflow.com/questions/51911075/how-to-check-awscli-and-compatible-botocore-package-is-installed) is causing pip install to use the boto3 cached during installing from requirements/dev.txt

.travis.yml Outdated
@@ -42,7 +42,7 @@ services:
- xvfb
install:
- pip install -r requirements/dev.txt
- pip install awscli==1.16.57 coveralls
- pip install --upgrade awscli boto3 botocore moto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update boto3, botocore, moto in requirements/dev.txt as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay if the changes are made in requirements/common.txt?
https://github.com/Cloud-CV/EvalAI/blob/master/requirements/common.txt#L2-L3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes.

django-ses==0.8.5
docker-compose==1.21.0
drfdocs==0.0.11
drf-yasg==1.11.0
kubernetes==10.0.1
moto==1.3.8
moto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moto should be in dev

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, since it was in common you can keep it here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay!

@nikochiko
Copy link
Contributor Author

I have updated the required deps: awscli, boto3, botocore and moto
I have also verified that they are working by writing some sample aws_utils tests. I think the changes in the PR have been tested well and the fix is working. 👍

I have removed the additional tests now.

@nikochiko
Copy link
Contributor Author

As pointed out by @yashdusing the reason the travis build was failing was because the tests in submission_worker were creating an actual boto3 object and not one mocked with moto. This was solved by moving the mock_sqs decorator to base class.

@yashdusing
Copy link
Contributor

LGTM 👍

@yashdusing
Copy link
Contributor

@Ram81 @vkartik97 @Sanji515 @RishabhJain2018 please review 😄

Copy link
Member

@Ram81 Ram81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@stale
Copy link

stale bot commented Mar 19, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactivity label Mar 19, 2020
@stale
Copy link

stale bot commented Mar 26, 2020

This pull request has been automatically closed as there is no further activity. Thank you for your contributions.

@stale stale bot closed this Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants