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

Add Support for MongoDB 5, 6, 7 PYMONGO 4.6.1 Drop Python 3.6 #6079

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

philipphomberger
Copy link

@philipphomberger philipphomberger commented Dec 1, 2023

This is a breaking Change because it drop Python 3.6. But I think that is the plan for Update 3.9.

  1. Update PYMONGO to 4.6.1
  2. Update mongoengine to 0.27.0
  3. Remove Some Tets and Code because ssl_match_hostname parameter not exist in newer Versions.

I have tested it on my dev servers with 3 Different Mongo DB Setups:

  1. mongodb flex Replika Set on Stackit Cloud Service (With SSL / User / Password)
  2. mongo db single Stackit Cloud Service (With SSL / User / Password)
  3. mongodb 7 local installation on devserver (Without SSL, Without User, Without Password)

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Dec 1, 2023
@philipphomberger
Copy link
Author

@armab Any Idea what I need to do to fix that ? I have changed the circleci image to 3.8 in my feature branche. https://app.circleci.com/pipelines/github/StackStorm/st2/4308/workflows/1222bb7c-0735-48ee-a44a-80c795f3c6ad/jobs/16112

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Good effort!
Please split changes into a two dedicated PRs.

One is about removing Python3.6 and another one about MongoDB (+pymongo) changes.
That'll help keeping things organized, good for history, testing and separates concerns.

@nzlosh
Copy link
Contributor

nzlosh commented Jan 17, 2024

According to the migration documentation ssl_match_hostname functionality was not dropped but replaced by tlsAllowInvalidHostnames https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html#renamed-uri-options

@philipphomberger
Copy link
Author

According to the migration documentation ssl_match_hostname functionality was not dropped but replaced by tlsAllowInvalidHostnames https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html#renamed-uri-options

okey. so id would be better to add this tlsAllowInvalidHostnames to replace the old ssl_match_hostname right ?

@nzlosh
Copy link
Contributor

nzlosh commented Jan 17, 2024

I think the work for adding support for mongodb 5+ will require working through the official migrate-to-pymongo4 document, checking the code base across the St2 repos (st2, orquesta, rbac, ldap, perhaps others) and update functions/methods/classes that reference the old v3 API.

The document recommends using pymongo 3.12 and turn on deprecation warnings to easily spot of the deprecated code. There's also the fact that it should not be tested against python 3.6, but rather python3.8, which is to be our lowest supported version for the 3.9.0 release.

We have the choice of keeping ssl_* configuration names and transpose them to tls equivalent so that users don't need to adjust configuration files or update to use tls to reflect the correct name of the underlying technology being used. Perhaps keeping ssl and tls versions for st2 v3.9.0 and with deprecation warnings of ssl version being removed in st2 v3.10

@philipphomberger
Copy link
Author

I think the work for adding support for mongodb 5+ will require working through the official migrate-to-pymongo4 document, checking the code base across the St2 repos (st2, orquesta, rbac, ldap, perhaps others) and update functions/methods/classes that reference the old v3 API.

The document recommends using pymongo 3.12 and turn on deprecation warnings to easily spot of the deprecated code. There's also the fact that it should not be tested against python 3.6, but rather python3.8, which is to be our lowest supported version for the 3.9.0 release.

We have the choice of keeping ssl_* configuration names and transpose them to tls equivalent so that users don't need to adjust configuration files or update to use tls to reflect the correct name of the underlying technology being used. Perhaps keeping ssl and tls versions for st2 v3.9.0 and with deprecation warnings of ssl version being removed in st2 v3.10

Thanks for the explanation. I thought it was too easy to just pull up the version.

@cognifloyd
Copy link
Member

I haven't read the PR, so this is a drive by comment after our TSC meeting.

Our codebase currently supports:

# mongoengine 0.24.0 has breaking changes to support pymongo 4.0
mongoengine>=0.21.0,<0.24.0
# pymongo 3.13 has backports of APIs from pymongo 4 to help w/ migration
pymongo>=3.11.0,<3.13.0

So, to update mongoengine/pymongo we need to migrate our code base based on:
https://docs.mongoengine.org/changelog.html#changes-in-0-24-0
https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html
https://pymongo.readthedocs.io/en/stable/changelog.html#changes-in-version-4-0

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ philipphomberger
❌ Philipp Homberger


Philipp Homberger seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants