search

opensearch-project / opensearch-py

Python Client for OpenSearch
https://opensearch.org/docs/latest/clients/python/
Apache License 2.0
338 stars 170 forks source link

Add support for urllib3 v2 #719

Closed iherasymenko closed 5 months ago

iherasymenko commented 5 months ago

Description

Allows using urllib3 v2 if Python version is >= 3.10. The same technique is applied in https://github.com/boto/botocore/pull/3141/files.

Issues Resolved

https://github.com/opensearch-project/opensearch-py/issues/628

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

saimedhi commented 5 months ago

Hello @iherasymenko, thank you for your contribution. Could you kindly include a few pertinent tests, please?

dblock commented 5 months ago

Maybe we can introduce an integration test that uses each version of the lib?

james-certn commented 5 months ago

Naive and hardline thinking: why preserve backward urllib3 compatibility in opensearchpy at all? If someone is tied to an old version of urllib3, they can use an older version of opensearchpy.

I get the desire for flexibility and supporting the most users possible, giving the latest to all users - but at some point the overhead isn't worth it, surely.

That aside, how would one best test this PR's changes in opensearchpy?

dblock commented 5 months ago

Naive and hardline thinking: why preserve backward urllib3 compatibility in opensearchpy at all? If someone is tied to an old version of urllib3, they can use an older version of opensearchpy.

I get the desire for flexibility and supporting the most users possible, giving the latest to all users - but at some point the overhead isn't worth it, surely.

That's all there is to it, flexibility. Generally if something is possible we tend not to break it to reduce downstream pain. If there's a good reason not to, or it's too hard to maintain, then we can drop it.

That aside, how would one best test this PR's changes in opensearchpy?

Since we're explicitly saying that we support multiple incompatible dependencies I think we need to introduce some way of doing integration tests where one vs. another version of the library is loaded. I don't know how to do this in Python best, in my Ruby experience we make separate Gemfile's in subfolders and run rspec against those (example).

If this is hard to do, I would be open to merging this as is. Let's get your questions answered on the compatible versions, I think those make a lot of sense.

iherasymenko commented 5 months ago

@james-certn The opensearch-py library is frequently (if not always) used together with the AWS Python SDK (https://github.com/boto/botocore/). Preferably, this library stays compatible with the Python 3.9 Lambda deployments, i.e. we don't pin the version of urllib3 to >2.x (as the urllib3 version there is pinned to 1.x) and, at the same time, we don't leave the version of urllib3 unpinned (>1.x) as it caused https://github.com/opensearch-project/opensearch-py/issues/628.

@dblock @saimedhi I'm thinking about adding a separate Integration Tests job that uses the public.ecr.aws/lambda/python:3.9 container so that we can make sure in a Python 3.9 environment, the right version gets picked up and this version does not conflict with the bundled boto3/botocore AWS Python SDK. Would this be a reasonable testing approach?

dblock commented 5 months ago

@dblock @saimedhi I'm thinking about adding a separate Integration Tests job that uses the public.ecr.aws/lambda/python:3.9 container so that we can make sure in a Python 3.9 environment, the right version gets picked up and this version does not conflict with the bundled boto3/botocore AWS Python SDK. Would this be a reasonable testing approach?

I would start simpler and definitely not attempt to actually make AWS calls (we want to do that, but setting up infrastructure that runs permanently is currently difficult, we need credits, etc.). Given a certain configuration (e.g. in a Pipfile), you end up with a specific version of the dependent library, hard-coded in a Python test.

james-certn commented 5 months ago

I'm not sure that I've seen tests in projects for this kind of requirements configuration. Not that I'm against the idea, but it does seem (subjectively) irregular. Maybe I just don't get out enough. :)

iherasymenko commented 5 months ago

@dblock Thanks. I updated the PR. I'll try to allocate some time to work on the tests this week.

james-certn commented 5 months ago

Sorry for the Github noise, but when would a release around this be planned? Right now, the 2.5.0 versioned package as is does block a subset of users from leveraging the latest and greatest.

A ballpark of a day, a week, a month - that's all I'm asking for. ;) Thank you!

saimedhi commented 5 months ago

Sorry for the Github noise, but when would a release around this be planned? Right now, the 2.5.0 versioned package as is does block a subset of users from leveraging the latest and greatest.

A ballpark of a day, a week, a month - that's all I'm asking for. ;) Thank you!

Hello @james-certn, could you please create an issue requesting a 2.6.0 release? Similar to issues #561 and #695. Thank you!