search

optimizely / python-sdk

Python SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/python-sdk
Apache License 2.0
32 stars 36 forks source link

Drop unused pyopenssl & cryptography dependencies #434

Closed PeterJCLaw closed 4 months ago

PeterJCLaw commented 5 months ago

Thanks for providing this package. It looks like this package pins (ranged) versions for cryptography and pyopenssl, yet doesn't actually use those packages (directly or transitively). This creates unnecessary dependency locks for consumers of this package as they're forced to install those packages even if nothing else is using them.

In turn, this means that consumers of this package may be exposed to security vulnerabilities in those packages due to their inclusion in this package.

If these packages are truly unused (I wasn't able to find a reference to them in the source of this project, nor are they mentioned by any of the other packages this one depends upon), please could they be removed from the package dependencies and an updated package be published?

PeterJCLaw commented 5 months ago

@Mat001 maybe (as you seem to be a recent contributor) is there anything I can do to help progress this? I'd be happy to put together a PR, though that feels like the easy part here -- I'm not sure what testing you'd want to do to validate the change nor if there's other concerns which lead to these packages being included (I couldn't find much in the git history).

Tamara-Barum commented 5 months ago

@PeterJCLaw Thanks for the call out- I have our Py eng taking a deeper look. we are opening an internal ticket and will prioritize and share updates here. Internal Ticket: FSSDK-10317

Mat001 commented 4 months ago

@PeterJCLaw Yeah, thx for reaching out. I have a PR out, you can see it. I think your point is valid. Do you remember or have been familiar with requests[security] extras from requests library? We used to use then prior to 2020. It included PyOpenSSL, cryptograph. But then Python stdlib started to natively support SSL/TLS (especially in older Py versions). So we removed requests[security] and basically replaced it / broke it down into what it contained. But I missed that PyOpenSSL and cryptography were no longer required.

Test have passed. PR is in review. I think we'll make a patch release.

Mat001 commented 4 months ago

@PeterJCLaw We've pushed out a patch release with your suggestion: https://github.com/optimizely/python-sdk/releases/tag/5.0.1 That is for the latest version of python-sdk 5.0.1. You will of course need to upgrade / use this version to get this change. Hope that resolves it! Thx for pointing this out and let us know if you spot anything else. Cheers

Mat001 commented 4 months ago

Closing.