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 python 3.10 compatibility #108

Merged
merged 3 commits into from
Mar 14, 2022

Conversation

lalmeras
Copy link
Contributor

  • vendored requests not compatible with python 3.10
  • vendored requests added because of disable pyopenssl for ovh to fix "EPIPE"
    that seems to be fixed by requests 2.11.0
  • so vendored requests is dropped and replaced by a adequate requests
    dependency
  • all vendored requests references are dropped
  • changelog proposal for next version

This is an alternate proposal to handle python 3.10 compatibility. It implements my proposal done on #106 to drop vendored requests as it seems to me that an updated requests library is a better way to fix an pyopenssl pooling issue.

Fix the python 3.10 issue:

envs/py3/lib/python3.10/site-packages/ovh/client.py:49: in <module>
    from .vendor.requests import request, Session
envs/py3/lib/python3.10/site-packages/ovh/vendor/requests/__init__.py:58: in <module>
    from . import utils
envs/py3/lib/python3.10/site-packages/ovh/vendor/requests/utils.py:30: in <module>
    from .cookies import RequestsCookieJar, cookiejar_from_dict
envs/py3/lib/python3.10/site-packages/ovh/vendor/requests/cookies.py:164: in <module>
    class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping):
E   AttributeError: module 'collections' has no attribute 'MutableMapping'

* vendored requests not compatible with python 3.10
* vendored requests added because of disable pyopenssl for ovh to fix "EPIPE"
  that seems to be fixed by requests 2.11.0
* so vendored requests is dropped and replaced by a adequate requests
  dependency
* all vendored requests references are dropped
* changelog proposal for next version
@BastianZim
Copy link

Hi @rbeuque74 apologies for pinging you directly but since you're one of the maintainers here, I wanted to ask if there is any progress with this PR?

I'm currently in the progress of adding ovh to conda-forge and the vendored requests is making this quite difficult. Additionally, it is missing some bug fixes that are available in newer versions so it would be great if this could be merged?

@BastianZim
Copy link

@rbeuque74 @geoffreybauduin Gentle ping again

@BastianZim
Copy link

Thanks @geoffreybauduin If you have the necessary rights, would you mind merging this and making a new release?

@geoffreybauduin
Copy link
Member

@BastianZim while I would be happy to move forward and help you fix this issue, you are now in the hands of @rbeuque74 to move forward. I'll contact him and see how we can set this up in the near future.

@BastianZim
Copy link

Great, thank you!

ovh/client.py Outdated
pass
from requests import request, Session
from requests.packages import urllib3
from requests.exceptions import RequestException

# Disable SNI related Warning. The API does not rely on it
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Disable SNI related Warning. The API does not rely on it

I will have to remove that one, as the API do now rely on it.

Choose a reason for hiding this comment

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

@rbeuque74 Thanks for the review. Does that need to be done in this PR or can this be merged?
Sorry for asking so much, I would just need this PR downstream. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Il push a new commit to remove comment and SNI warning deactivation.

@geoffreybauduin
Copy link
Member

We are currently adapting our CI pipeline, since this repository was still using travis-ci.org, in order to be able to push a new version to pypi.

Once it is setup we will move forward. Sorry for the delay

Signed-off-by: Romain Beuque <556072+rbeuque74@users.noreply.github.com>
@rbeuque74
Copy link
Member

Setup of the CI pipeline took longer as expected, as we stumble upon a lot of incompatibilities due to build libraries not compatible anymore with Python 2.7, 3.4 and 3.5.
Therefore, I released a v0.6.0 version, that doesn't include Python 3.10 compatibility.
Python 3.10 will come with v1.0.0 that will drop compatibility to Python 2.7, 3.4, 3.5.

@rbeuque74
Copy link
Member

Thanks for your contribution btw 🥇

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