triton-inference-server / client

Triton Python, C++ and Java client libraries, and GRPC-generated client examples for go, java and scala.
BSD 3-Clause "New" or "Revised" License
516 stars 224 forks source link

Revert urllib3 version pin #648

Open tokoko opened 1 month ago

tokoko commented 1 month ago

Hi. We are trying to integrate with OIP model servers over at feast feature store and need to add mlserver and tritonclient as optional dependencies. The problem is that we also already depend on snowflake-connector-python which still has a strict urllib3<2.0.0 requirement for python 3.9. I saw that urllib3 version pin here was only to avoid vulnerability reports. #457 Not sure which vulnerability that was referring to, but seems like urlib3 plan to ship security fixes for v1 still. Is is possible to revert the version pin? Or maybe allow something like (>=1.26.18<2 or >=2.0.7). Thanks

debermudez commented 1 month ago

@nvda-mesharma @mc-nv in this PR: https://github.com/triton-inference-server/client/pull/457, we updated the urllib3 requirement. Is this something we can investigate?

mc-nv commented 1 month ago

CVE-2023-45803 prevent locking version as urllib3<2.0.0.

tokoko commented 1 month ago

@mc-nv CVE says that This issue has been addressed in versions 1.26.18 and 2.0.7. I know it's not pretty, but are you opposed to pinning like this urllib3>=1.26.18,!=2.0.2,!=2.0.3,!=2.0.4,!=2.0.5,!=2.0.6? (Or maybe there's some other way to write this.. but you get the idea)

tokoko commented 1 month ago

Sorry to bother you guys once again. Have you had an opportunity to look into this? It's a really simple fix that will avert a dependency hell for us.

debermudez commented 1 month ago

Its not bothering us. We just have to be careful with the CVE issue. Let me ping one more person. @nvda-mesharma is this something we can accommodate?

mc-nv commented 1 month ago

Dear customer we understand your request and have to clarify following for you.

According to Python Packaging User Guide#Version Specifier documentation both version are valid as you mentioned above. Question that raise for us, is why https://pypi.org/project/urllib3 project carry two different semantic versions (see https://semver.org/ )? Suppose it cause because these two version can carry different set of API with a similar functionality.

We may not be impacted, but it will require rebuild entire product and make sure that none of the other customers is impacted.

tokoko commented 1 month ago

@mc-nv Hey, thanks for the clarification. Regarding semver, I'll note here a quote from urllib3 v2 migration guide:

The primary goal for migrating to urllib3 v2.x should be to ensure your package supports both urllib3 v1.26.x and v2.0 for some time

And their recommended version specifier during the migration is "urllib3>=1.26,<3" (which will have to be modified a bit because of the CVE, of course).

tokoko commented 3 weeks ago

Hey guys. Just pinging if there's any update on this?

debermudez commented 2 weeks ago

@nvda-mesharma any idea what is happening on this? I am out of the loop on this now

aW3st commented 22 hours ago

We are also running into this in an application that uses boto3. Boto requires urllib3<2.0.0.