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

Remove vendored version of requests in botocore and bump version #739

Closed
wants to merge 4 commits into from

Conversation

kevchentw
Copy link

Description of Change

New version of botocore remove vendored version of requests, therefore it should remove all code using botocore.vendored.requests.

I copied the CaseInsensitiveDict from requests to aiobotocore to use this structures. The alternative way it to put requests into requirements, however aiobotocore didn't use other reqeusts function, so I think it is unnecessary.

To reproduces this bug, update botocore to >= 1.13.0, and make requuests with aiobotobore.
The following error will generated.

from botocore.vendored.requests.structures import CaseInsensitiveDict
ModuleNotFoundError: No module named 'botocore.vendored.requests.structures'

ref:

bump version

  1. botocore==1.12.252 -> botocore==1.13.2
  2. boto3==1.9.252 -> boto3==1.10.2
  3. awscli==1.16.262 -> awscli==1.16.266

Assumptions

Checklist for All Submissions

  • I have added change info to CHANGES.txt
    • [V ] Detailed description of issue
    • [V ] Alternative methods considered (if any)
    • How issue is being resolved
    • How issue can be reproduced

Checklist when updating botocore and/or aiohttp versions

  • I have read and followed CONTRIBUTING.rst
  • I have updated test_patches.py where/if appropriate (also check if no changes necessary)
  • I have ensured that the awscli/boto3 versions match the updated botocore version

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2019

This pull request introduces 2 alerts when merging 5eb29ce into 957c91b - view on LGTM.com

new alerts:

  • 1 for Wrong number of arguments in a call
  • 1 for Wrong number of arguments in a class instantiation

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2019

This pull request introduces 5 alerts when merging 074e088 into 957c91b - view on LGTM.com

new alerts:

  • 3 for __eq__ not overridden when adding attributes
  • 1 for Wrong number of arguments in a call
  • 1 for Wrong number of arguments in a class instantiation

@thehesiod
Copy link
Collaborator

cool, thank you! however we should do what botocore is doing and not copy code from them. Also, while working on #659 I found that somehow we missed a bunch of changes to make use match botocore, so we should pull those as well when bumping botocore base version. Thanks for your help!

@thehesiod
Copy link
Collaborator

closing in favor of #743 as we shouldn't need to copy the case insensitive dict as that PR shows.

@thehesiod thehesiod closed this Nov 10, 2019
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.

2 participants