Skip to content

Conversation

@jonshea
Copy link
Contributor

@jonshea jonshea commented May 10, 2023

There was an upper version bound on pymongo of <4.0.0 because versions 4.0.0+ removed the ssl_cert_reqs parameter from client connection options. However, comparing the pymongo 3.7.0 docs (https://pymongo.readthedocs.io/en/3.7.0/examples/tls.html#certificate-verification-policy) to the 4.3.3 docs
(https://pymongo.readthedocs.io/en/4.3.3/examples/tls.html#certificate-verification-policy) it seems that ssl_cert_reqs has been effectively renamed to tlsAllowInvalidCertificates.

This PR removes upper version bound on pymongo, and adds a test to set tlsAllowInvalidCertificates for pymongo 4.0.0 and later, while continuing to use ssl_cert_reqs for earlier versions of pymongo.

There was an upper version bound on pymongo of `<4.0.0` because versions
4.0.0+ removed the `ssl_cert_reqs` parameter from client connection
options. However, comparing the pymongo 3.7.0 docs
(https://pymongo.readthedocs.io/en/3.7.0/examples/tls.html#certificate-verification-policy)
to the 4.3.3 docs
(https://pymongo.readthedocs.io/en/4.3.3/examples/tls.html#certificate-verification-policy)
it seems that `ssl_cert_reqs` has been effectively renamed to
`tlsAllowInvalidCertificates`.

This PR removes upper version bound on pymongo, and adds a test to set
`tlsAllowInvalidCertificates` for pymongo 4.0.0 and later, while continuing
to use `ssl_cert_reqs` for earlier versions of pymongo.
@jonshea
Copy link
Contributor Author

jonshea commented May 10, 2023

I am not certain about what to do for testing. It isn’t clear to me that any existing tests actually connect to a live mongodb instance?

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Seems to be working now

@eladkal eladkal marked this pull request as ready for review May 11, 2023 06:17
@eladkal eladkal merged commit d4dc734 into apache:main May 11, 2023
@potiuk
Copy link
Member

potiuk commented May 11, 2023

Nice!

@potiuk
Copy link
Member

potiuk commented May 11, 2023

I am not certain about what to do for testing. It isn’t clear to me that any existing tests actually connect to a live mongodb instance?

Actually we have integration tests with live mongo (dockerised) and they passed:

https://github.com/apache/airflow/actions/runs/4945112005/jobs/8841910985#step:11:596

tests/integration/providers/mongo/sensors/test_mongo.py

Not sure though if they are comprehensive enough, so maybe good if you take a look as well?

It's rather easy to run them locally with breeze:

https://github.com/apache/airflow/blob/main/TESTING.rst#airflow-integration-tests

@potiuk
Copy link
Member

potiuk commented May 11, 2023

Though in your PR mongo has not been upgraded actually (because it is held back) and the constraints had not updated it:

https://github.com/apache/airflow/actions/runs/4940266515/jobs/8837512485?pr=31189#step:5:17770

But this is actually because of apache-beam holding it back:

apache-beam 2.46.0 requires pymongo<4.0.0,>=3.8.0, but you have pymongo 4.3.3 which is incompatible.

And we will find out today/tomorrow if all is fine, because `apache-beam 2.47 had just been released and we are about to get it upgraded as part of the job in #30067

This apache-beam release is something we've been waiting for some time as it held back a number of our dependencies :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants