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 #1829

Merged
merged 2 commits into from
Oct 21, 2019
Merged

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Sep 20, 2019

This PR removes the vendored requests from botocore.

NOTE: This PR is not going to be merged immediately. This is
being sent in advance so people can use this branch to test. The
planned date to merge this is 10/21/19.

I've left the exceptions for requests and its vendored urllib3 so that anyone
that needed to catch these exceptions (that we used to leak) will not be broken.

A bit of history on this change.

The original version of botocore, before GA, had requests as a direct
dependency. In the leadup to GA, we decided to vendor the requests library
at the request of package distro maintainers. We frequently needed the latest
version of requests, and package maintainers weren't able to immediately
upgrade requests globally, given it's popularity and widespread usage.

The 1.0.0 GA release back in 2015 continued to keep the vendored version of
requests in botocore.

Four years later, things have changed. First, the requests feature set
stabilized. We no longer need to constantly be pulling in the latest version
of requests for new features. We also kept needing more low level
functionality that was more accessible in urllib3 (which requests depends on).
Last year we decided to put in an abstraction over the http library we use so
that we could migrate from requests to urllib3 while also allowing pluggable
http clients.

Once this change was merged last year, botocore no longer uses the vendored
version of requests, but we decided to keep it in place instead of immediately
removing it. Yes, it is a vendored library, and even namespaced as such, but
it's still technically possible to import the vendored library of requests and
use it directly in your code. This vendored library was never something we
publicly documented or ever supported people directory using.

We wanted to give people adequent time to migrate away from this usage
before removing the vendored requests entirely.

Timeline of the deprecation

This deprecation has been in the works for a while, here's a timeline of
everything:

Additionally, python 3.8 is being released on October 21st, 2019, and there
are changes that will break our vendored version of requests. This means that
we need to do something or else python 3.8 will not work with botocore.

If you tried to use this deprecated package, the warning looks like this:

>>> import botocore.vendored.requests
>>> botocore.vendored.requests.get('https://aws.amazon.com')
./botocore/vendored/requests/api.py:67: DeprecationWarning: You are using the get() function from 'botocore.vendored.requests'.  This is not a public API in botocore and will be removed in the future. Additionally, this version of requests is out of date.  We recommend you install the requests package, 'import requests' directly, and use the requests.get() function instead.
  DeprecationWarning

How to fix your code

If you were previously using our vendored version of requests, you can migrate
away from this by installing requests into your python environment and
importing requests directly:

Before


from botocore.vendored import requests

response = requests.get('https://...')

After

$ pip install requests
import requests

response = requests.get('https://...')

Please let us know if you have any questions or concerns.

Related issues:

The exceptions are left in place for backwards compatibility.
@codecov-io
Copy link

codecov-io commented Sep 20, 2019

Codecov Report

Merging #1829 into develop will increase coverage by 0.35%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1829      +/-   ##
===========================================
+ Coverage     92.2%   92.55%   +0.35%     
===========================================
  Files           53       53              
  Lines         9958     9958              
===========================================
+ Hits          9182     9217      +35     
+ Misses         776      741      -35
Impacted Files Coverage Δ
botocore/utils.py 97.98% <0%> (+0.16%) ⬆️
botocore/credentials.py 98.85% <0%> (+0.34%) ⬆️
botocore/hooks.py 96.38% <0%> (+0.4%) ⬆️
botocore/handlers.py 96.92% <0%> (+0.47%) ⬆️
botocore/endpoint.py 99.29% <0%> (+0.7%) ⬆️
botocore/compat.py 93.92% <0%> (+4.97%) ⬆️
botocore/awsrequest.py 98.36% <0%> (+5.9%) ⬆️

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 92063ce...084e90e. Read the comment docs.

HTTPSConnectionPool,
connection_from_url
)

from . import exceptions
Copy link

Choose a reason for hiding this comment

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

This seems odd:

  • vendored requests are removed
  • vendored urllib3 is removed
  • but vendored urllib3.exceptions remains?

Copy link

Choose a reason for hiding this comment

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

re: "for those who used these features"
I think perhaps this can be worked around by e.g.:

# vendored/__init__.py
import requests

which would ensure that user can still from botocore.vendored.requests.urllib3.exceptions import X
In any case, if someone were to catch e.g. HTTPError, it needs to be same HTTPError is thrown in the dependencies, right?

Copy link
Contributor

@joguSD joguSD Sep 23, 2019

Choose a reason for hiding this comment

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

It is a little strange, but let me provide a little more context on this. In earlier versions of botocore we leaked exceptions from requests which meant that in some cases users may have needed to import an exception class similar to what you show:

from botocore.vendored.requests.urllib3.exceptions import X

In the migration to urllib3 we generally followed the exception structure in requests/urllib3 and mapped them to our own exception classes. Leaving the exception classes is simply to ensure users using a line similar to the above will continue to work (as closely as possible) and avoid a hard import error.

We have considered the latter suggestion but as we no longer depend on requests it's a little more tricky than just doing the import as requests may or may not be present. We also have no control over the version of requests which leaves the possibility for something to change in requests breaking the SDK (though this is probably unlikely). I know that requests did a similar change when they removed their vendored urllib3 but things are pretty different here since we no longer depend on requests.

@aayushi03
Copy link

pip install requests
I am using the above statement while using it with ibm boto3 but still getting the same error. Could you help me out in it ? Please help

Code to download file from IBM cos bucket to local storage using serverless function:

import requests
import ibm_boto3
from ibm_botocore.client import Config
import os,os.path
import pathlib

cos = ibm_boto3.resource(service_name='s3',
ibm_api_key_id=',
ibm_service_instance_id='',
config=Config(signature_version='oauth'), endpoint_url='https://s3.eu-gb.cloud-object-storage.appdomain.cloud')

try:
res = cos.meta.client.download_file(Bucket='cloud-college-bucket0',Key='abc.txt',Filename='D:\ibm-cloud')

except Exception as e:
print(Exception, e)
else:
print('File Downloaded')

Error:

DeprecationWarning: You are using the post() function from 'ibm_botocore.vendored.requests'. This is not a public API in ibm_botocore and will be removed in the future. Additionally, this version of requests is out of date. We recommend you install the requests package, 'import requests' directly, and use the requests.post() function instead.\n DeprecationWarning\nTraceback (most recent call last):\n File "/action/1/src/exec__.py", line 43, in \n from main__ import main as main\nImportError: cannot import name 'main' from 'main__' (/action/1/src/main__.py)\n"

@katzdm
Copy link

katzdm commented Oct 8, 2019

pip install requests
I am using the above statement while using it with ibm boto3 but still getting the same error. Could you help me out in it ? Please help

Code to download file from IBM cos bucket to local storage using serverless function:

import requests
import ibm_boto3
from ibm_botocore.client import Config
import os,os.path
import pathlib

cos = ibm_boto3.resource(service_name='s3',
ibm_api_key_id=',
ibm_service_instance_id='',
config=Config(signature_version='oauth'), endpoint_url='https://s3.eu-gb.cloud-object-storage.appdomain.cloud')

try:
res = cos.meta.client.download_file(Bucket='cloud-college-bucket0',Key='abc.txt',Filename='D:\ibm-cloud')

except Exception as e:
print(Exception, e)
else:
print('File Downloaded')

Error:

DeprecationWarning: You are using the post() function from 'ibm_botocore.vendored.requests'. This is not a public API in ibm_botocore and will be removed in the future. Additionally, this version of requests is out of date. We recommend you install the requests package, 'import requests' directly, and use the requests.post() function instead.\n DeprecationWarning\nTraceback (most recent call last):\n File "/action/1/src/exec__.py", line 43, in \n from main__ import main as main\nImportError: cannot import name 'main' from 'main__' (/action/1/src/main__.py)\n"

Seems to be a separate repository - Might want to file the issue there.
https://github.com/IBM/ibm-cos-sdk-python-core/tree/master/ibm_botocore

@selik
Copy link

selik commented Oct 16, 2019

NOTE: This PR is not going to be merged immediately. This is being sent in advance so people can use this branch to test. The planned date to merge this is 10/21/19.

Additionally, python 3.8 is being released on October 21st, 2019

@jamesls since Python 3.8 has already been released, could we merge this a few days earlier than planned?

@itizir
Copy link

itizir commented Oct 21, 2019

Might be good to update this: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-lambda-function-code-cfnresponsemodule.html
and get this PR in (gene1wood/cfnresponse#2) before things break...

jamesls added a commit that referenced this pull request Oct 21, 2019
PR #1829

* remove-vendor:
  Update tests to not rely on vendored requests
  Remove vendored requests
@jamesls jamesls merged commit 084e90e into boto:develop Oct 21, 2019
@jamesls
Copy link
Member Author

jamesls commented Oct 21, 2019

Merged! We'll follow up with the docs team to get that code snippet updated.

@mcallaghan-bsm
Copy link

Please update headline to also state urllib3 is removed.

@brysontyrrell
Copy link

brysontyrrell commented Oct 24, 2019

We're now seeing this error now when using cfnresponse:

send(..) failed executing requests.put(..): module 'botocore.vendored.requests' has no attribute 'put'

I have a PR open on the repo behind the PyPI to fix this but hasn't gotten a response yet:
gene1wood/cfnresponse#2

@purlaksh
Copy link

purlaksh commented Oct 31, 2019

Is python 3.8 available for lambda now? We do not see it on the console?

@dxciberaws
Copy link

dxciberaws commented Nov 4, 2019

We're now seeing this error now when using cfnresponse:

send(..) failed executing requests.put(..): module 'botocore.vendored.requests' has no attribute 'put'

I have a PR open on the repo behind the PyPI to fix this but hasn't gotten a response yet:
gene1wood/cfnresponse#2

+1. I believe this happens because in my case the code updates botocore using pip install from code, to be able to use updated API features. Probably the botocore version in standard Lambda environment is not updated and would not suffer from this issue, but then we would not be able to use the newer features.
Guess we'll have to move from inline code to ZIP packages where we install the requests package and include the cfnresponse.py module. At least until the standard Python Lambda environment is updated to this botocore version and a new cfnresponse.py module.

@thangavel-projects
Copy link

thangavel-projects commented Nov 18, 2019

I followed the doc https://aws.amazon.com/blogs/developer/removing-the-vendored-version-of-requests-from-botocore/

and combining the requests to "pip install requests awscli boto3 pytest --upgrade" will that works? For me, Jeankins job was successful and the same issue while invoking REST call.

Any idea?

Thanks in advance!

@jeshan
Copy link

jeshan commented Nov 24, 2019

Won't this break lambda functions that's using vendored?

@meshuga
Copy link

meshuga commented Feb 7, 2020

Would be good to provide a migration guideline for Lambda breaking change, so that people can use
e.g. urllib3 provided along with the environment [1] instead of the deprecated botocore.vendored.requests.I use that code in some of the Lambdas and assume a lot of other people also use that module as well.

[1] https://gist.github.com/gene1wood/4a052f39490fae00e0c3

@chrisdlangton
Copy link

chrisdlangton commented Feb 10, 2020

@meshuga have you tied the direction from "How to fix your code"? If so, what is your experience?

@meshuga
Copy link

meshuga commented Feb 10, 2020

@chrisdlangton I did look at the How to fix your code but the approach is not acceptable for me since it degrades the ease of usage of AWS Lambda. I have code in AWS Lambda without any dependencies and don't want to add any library for sake of making HTTP requests, since #1829 (comment).

@Wulfson
Copy link

Wulfson commented Feb 19, 2020

This is pretty bothersome. I have a number of very small Lambdas that currently have zero dependencies because they're able to make use of the vendored requests lib, and now I'm going to have to do full package deploys just to update them when I could have used the AWS Console's editor previously.

@a-feld
Copy link

a-feld commented Feb 20, 2020

I'm not sure what the current story is on project maintenance for requests (see psf/requests#5149 for some of the discussion) but for those interested in a migration path, it's probably worth using the more actively maintained urllib3 for synchronous HTTP/1 calls.

botocore (which currently lists urllib3 as a package dependency) is installed in all Python lambda environments. This means that urllib3 is also available for import without any additional packaging requirements. As of today, it looks like all of the Python lambda environments have urllib3 version 1.25.7 installed.

Example lambda using urllib3

import urllib3
http = urllib3.PoolManager()


def lambda_handler(event, context):
    r = http.request('GET', 'https://example.org/')
    return {
        'statusCode': r.status,
        'body': r.data,
    }

Side note: urllib3 isn't guaranteed to be installed by AWS anywhere in the docs, it's possible it can disappear in the future.

@drambed
Copy link

drambed commented Feb 25, 2020

cfn-response module still uses vendored.requests
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-lambda-function-code-cfnresponsemodule.html
Is there a plan to fix the dependency in that module?
We are using this in several cloudformation created Lambdas that don't have enough free bytes to replace it inline.

@acsekhar2000
Copy link

As advised, we have removed - from botocore.vendored import requests - and using - import requests. But we are getting the below warning.

WARNING - botocore.vendored.requests.packages.urllib3.connectionpool - Connection pool is full, discarding connection: trp-usis-stage-us-east-1-mft-intermediariesanalytics-raw.s3.amazonaws.com

We are using boto3 version 1.7.35.

Do we have to upgrade our boto3 version too?

@ahmadfs
Copy link

ahmadfs commented Mar 30, 2020

Looks like the date was moved to Jan 2021 as per https://aws.amazon.com/blogs/compute/upcoming-changes-to-the-python-sdk-in-aws-lambda/.

I have tried using this approach and found a few posts on AWS Forums and comments in the same blog but still doesn't work. While I appreciate moving the date out to Jan, 2021, those of us who would like to fix this problem right now are stuck because the layer approach doesn't work.

morenol pushed a commit to eduNEXT/testeng-ci that referenced this pull request May 7, 2020
morenol pushed a commit to eduNEXT/testeng-ci that referenced this pull request May 8, 2020
Remove vendored requests from botocore boto/botocore#1829

Add constraint to ddt
morenol pushed a commit to eduNEXT/testeng-ci that referenced this pull request May 8, 2020
Remove vendored requests from botocore boto/botocore#1829

Add constraint to ddt
morenol pushed a commit to eduNEXT/testeng-ci that referenced this pull request Jun 29, 2020
Remove vendored requests from botocore boto/botocore#1829

Add constraint to ddt
@manhkhoa
Copy link

Hello everyone, i am using cfn-response module with nodejs. Do i need to update anything?

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.

DeprecationWarnings in vendored requests (unused) vendored requests is vulnerable to CVE-2018-18074