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

DeprecationWarnings in vendored requests #1615

Closed
mvanbaak opened this issue Nov 27, 2018 · 30 comments · Fixed by #1829
Closed

DeprecationWarnings in vendored requests #1615

mvanbaak opened this issue Nov 27, 2018 · 30 comments · Fixed by #1829
Labels
dependencies This issue is a problem in a dependency. enhancement This issue requests an improvement to a current feature.

Comments

@mvanbaak
Copy link

mvanbaak commented Nov 27, 2018

The vendored requests library is emitting DeprecationWarnings:

lib/python3.7/site-packages/botocore/vendored/requests/packages/urllib3/_collections.py:1: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
    from collections import Mapping, MutableMapping

This has been fixed upstream here: psf/requests#4501

Since I do not know how you handle the vendored requests, I have no idea how to make a PR to fix it. sorry.

Can someone pick this up? It's very annoying while running tests. Or give me a hint how to create a PR for the vendored requests, and I'll create one.

@JordonPhillips
Copy link
Contributor

What are you doing to trigger that? botocore moved off of using the vendored requests a while ago, it's only kept in for backwards compatibility of certain exceptions. I can't get the warning to trigger by using botocore itself.

@mvanbaak
Copy link
Author

Runnning pytest for a project that uses botocore is enough to trigger this warning.

@joguSD
Copy link
Contributor

joguSD commented Dec 3, 2018

I've confirmed this, using 3.7.1 and pytest. Simply importing a botocore session is enough to trigger this. I think we need to come up with a more long term plan for handling our vendored version of requests.

@joguSD joguSD added enhancement This issue requests an improvement to a current feature. dependencies This issue is a problem in a dependency. and removed closing-soon labels Dec 3, 2018
@schlomo
Copy link

schlomo commented Dec 4, 2018

Annoying for any pytest user, would be nice if you can fix it. Maybe just try importing from collections.abc and fall back to importing from collections? Python style is just try and catch error instead of checking for Python versions.

@zgoda-mobica
Copy link

Python 3.7.2, pytest-4.0.2

The only boto3 related imports in the whole code base:

import boto3
from boto3 import Session
from botocore.client import Config
from botocore.exceptions import ClientError

@TomAugspurger
Copy link

Here's a simple example

import warnings
warnings.simplefilter("error", DeprecationWarning)

import boto3

That raises with

$ python bug.py
Traceback (most recent call last):
  File "bug.py", line 4, in <module>
    import boto3
  File "/private/tmp/tmp/lib/python3.7/site-packages/boto3/__init__.py", line 16, in <module>
    from boto3.session import Session
  File "/private/tmp/tmp/lib/python3.7/site-packages/boto3/session.py", line 17, in <module>
    import botocore.session
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/session.py", line 29, in <module>
    import botocore.configloader
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/configloader.py", line 19, in <module>
    from botocore.compat import six
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/compat.py", line 25, in <module>
    from botocore.exceptions import MD5UnavailableError
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/exceptions.py", line 15, in <module>
    from botocore.vendored import requests
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/vendored/requests/__init__.py", line 53, in <module>
    from .packages.urllib3.contrib import pyopenssl
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/vendored/requests/packages/__init__.py", line 3, in <module>
    from . import urllib3
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/vendored/requests/packages/urllib3/__init__.py", line 10, in <module>
    from .connectionpool import (
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/vendored/requests/packages/urllib3/connectionpool.py", line 38, in <module>
    from .response import HTTPResponse
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/vendored/requests/packages/urllib3/response.py", line 9, in <module>
    from ._collections import HTTPHeaderDict
  File "/private/tmp/tmp/lib/python3.7/site-packages/botocore/vendored/requests/packages/urllib3/_collections.py", line 1, in <module>
    from collections import Mapping, MutableMapping
  File "<frozen importlib._bootstrap>", line 1032, in _handle_fromlist
  File "/usr/local/Cellar/python/3.7.2/Frameworks/Python.framework/Versions/3.7/lib/python3.7/collections/__init__.py", line 52, in __getattr__
    DeprecationWarning, stacklevel=2)
DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working

Version info:

boto3            1.9.73
botocore         1.12.73

@TomAugspurger
Copy link

botocore moved off of using the vendored requests a while ago

It seems to be imported at the top level in

from botocore.vendored import requests

Should that be in a

try:
    import requests
except ImportError:
    from botocore.vendored import requests

?

@therc
Copy link

therc commented Jan 16, 2019

I triggered a different, but related problem somewhere else:

File "exedir/botocore/vendored/requests/packages/urllib3/connectionpool.py", line 12, in <module>
    from Queue import LifoQueue, Empty, Full
ImportError: cannot import name LifoQueue

This is complicated to trigger, as I suspect it requires a) running on a case-insensitive FS (don't ask...) and b) having a queue.py (note the lower case) file somewhere in your search path.

Anyway, my problem is probably obviated by recent changes in source layout in urllib3 (1.23 and greater).

@rbu
Copy link

rbu commented Mar 28, 2019

from botocore.vendored import requests
Should that be in a

try:
    import requests
except ImportError:
    from botocore.vendored import requests

I think that's not correct. The vendored.requests import is used to support old code written against botocore when it still used the vendored requests that looks like this:

from botocore.vendored.requests.exceptions import ReadTimeout

try:
    ... some botocore client code ...
except ReadTimeout:
    ...

That code would specifically expect the vendored exception.

Unfortunately, code like this does not currently trigger a deprecation warning, so dropping compatibility right away might be a problem. You could simply remove everything except the vendored exceptions, port them and have them trigger an exception when imported through the vendored path.

@mcallaghan-bsm
Copy link

mcallaghan-bsm commented May 16, 2019

See boto/boto3#1968 for a full technical assessment.

@mcallaghan-bsm
Copy link

I would argue/propose that this is a bug rather than an enhancement - perhaps back in 2018 it was an enhancement / deprecation issue -- but time is ticking. Whether bug/enhancement, it is important to identify a milestone to ensure this is scoped and addressed in an upcoming release.

@iandow
Copy link

iandow commented May 23, 2019

To ignore the warning in pytest, use the -W flag, like this:
pytest -W ignore::DeprecationWarning

@mcallaghan-bsm
Copy link

Alternatively, if you're using configuration files, ignore like this: (e.g. with pytest.ini)

# https://docs.pytest.org/en/latest/warnings.html
filterwarnings =
    # https://github.com/boto/boto3/issues/1968
    ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working:DeprecationWarning

(as an example directly related to boto/boto3#1968)

@mvanbaak
Copy link
Author

Just a warning: This ignore is global, not specific to botocore.

@mcallaghan-bsm
Copy link

Can we get milestone commitment to have this (#1968) fixed? We don't want to leave this much longer.

@joaonc
Copy link

joaonc commented Jun 26, 2019

This fix is already in the latest urllib3: urllib3/urllib3#1325

botocore can simply update the urllib3 package included, or add a similar fix.

This is quite annoying..

@dimaqq
Copy link

dimaqq commented Aug 8, 2019

Ping :)

@mcallaghan-bsm
Copy link

Ping :)

This fix is already in the latest urllib3: urllib3/urllib3#1325

botocore can simply update the urllib3 package included, or add a similar fix.

This is quite annoying..

if we "just do this", are we worried about breaking anything? does this package have sufficient UT or other auto-test coverage to validate a bump to urllib?

@rbu
Copy link

rbu commented Aug 10, 2019

urllib3 and requests should not have their vendored copy updated, but removed. They are not used, instead botocore depends on the maintained packages.

The only reason the copy is kept around is to allow possibly existing like the one in #1615 (comment) to run without errors. Unfortunately, as far as I can see, code like this never raised a DeprecationWarning itself asking users to catching the new exception classes. Nevertheless, since 1.11.0 released on Aug 25, 2018, the Upgrade notes clearly state:

Upgrading to 1.11.0
---------------------
* The vendored versions of ``requests`` and ``urllib3`` are no longer being
  used and have been replaced with a direct dependency on upstream ``urllib3``
  and ``requests`` is no longer a dependency of ``botocore``.  While these
  vendored dependencies are still in the ``botocore`` package they should not
  be used as they will be removed in the future. Any code that imports from
  ``botocore.vendored.requests.*`` should be updated accordingly. Specifically,
  the use of ``botocore.vendored.requests.exceptions.*`` or
  ``botocore.vendored.requests.packages.urllib3.exceptions.*`` must be updated
  to the corresponding exception classes in ``botocore.exceptions``.

My +1 is for ripping it out entirely and bumping the version to 1.13 as the backwards-compatiblity now causes more harm than good.

@roshinv
Copy link

roshinv commented Aug 28, 2019

You can also ignore this warning by adding the following code in your conftest.py

def pytest_configure(config):
    config.addinivalue_line("filterwarnings", "ignore::DeprecationWarning")

@Jamim
Copy link

Jamim commented Aug 28, 2019

You can also ignore this warning by adding the following code in your conftest.py

Yeah, until your tests start crashing on Python 3.8 🤦‍♂️

@roshinv
Copy link

roshinv commented Aug 28, 2019

You can also ignore this warning by adding the following code in your conftest.py

Yeah, until your tests start crashing on Python 3.8 🤦‍♂

Isn't botocore supposed to fix this issue?

@jamesls
Copy link
Member

jamesls commented Sep 20, 2019

Hi, we are planning on removing the vendored requests (exception for the exceptions) from botocore in one month. Please let us know if you have any questions or concerns in the tracking PR: #1829

@ssbarnea
Copy link

Resolving one DeprecationWarning in ~1 year from when it was reported, that is clearly something to write on the annual review achievements section. It it would be up to me me I would fail to mention the DeprecationWarning and only state that it had 46 upvotes ;)

@zgoda-mobica
Copy link

Python 3.8.0 landed today.

@toolness
Copy link

toolness commented Oct 30, 2019

Hey, for folks still following this, it looks like there's still just a DeprecationWarning in 3.8, and the warning message says the relevant entries from collections will be removed in 3.9:

Python 3.8.0 (default, Oct 17 2019, 05:36:36)
...
>>> from collections import Mapping
<stdin>:1: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.9 it will stop working

The unit tests for my project still seem to work on 3.8. Obviously #1829 is still very much appreciated and I'll be moving to the new version of botocore ASAP, so I can stop squelching the warning--but it's nice to know that 3.8 doesn't seem to actually break anything, as far as I can tell.

@asherf
Copy link

asherf commented Oct 30, 2019

@toolness
Copy link

Heh, thanks for the clarification!

However, I just realized that I can't actually un-squelch the warning because #1865 still needs to be merged 😭 Ah well.

@aaronlevenstein
Copy link

Python 3.9 is out now what is the status on this?

@nateprewitt
Copy link
Contributor

Hi @aaronlevenstein, would you be able to clarify which "this" you're referring to? Requests has been removed from botocore for over two years. If you're seeing a separate issue, you may want to open a new ticket so we can better track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies This issue is a problem in a dependency. enhancement This issue requests an improvement to a current feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.