-
-
Notifications
You must be signed in to change notification settings - Fork 774
Upgrade pyyaml to the latest version #4510
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
Conversation
NOTE: Security vulnerability doesn't affect our code because we already use yaml.safe_load everywhere, but it's still good to update.
pyyaml to old version.
|
I'm wondering if it might be easier to simply maintain the upstream project. Edit: Meaning the flex project. |
|
@Kami I am now a maintainer for the flex project on GitHub and PyPI, and you should be added today or tomorrow. Instead of maintaining our own fork, lets just maintain the upstream fork. People are more likely to submit PRs to the upstream project than to our fork, so instead of carrying 100% of the maintenance burden, we can get community involvement. Also note that the upstream project already has issues and PRs stacked up already, so we have immediate community involvement instead of having to cultivate our own. I'll work on a PR for the upstream flex project. |
|
Upstream PR is here: pipermerriam/flex#213 Given that, I don't think we need our own fork anymore. 😄 |
|
Tweaked the PR to use PyYAML between That way, when PyYAML 4.2b gets yanked from PyPI it doesn't break our build, and we don't have to revisit this when the flex PR gets merged and released to PyPI. |
|
Great 👍 So we should be good to go once upstream change is merged and new version published to PyPi, right? |
|
Yep, exactly. |
This pull request updates pyyaml to the latest version.
Keep in mind that the security issue which has been addressed with a safer default doesn't affect any of our code since we already use
yaml.safe_loadeverywhere, but it's still a good idea to upgrade to avoid pack content or similar using potentially unsafeyaml.load.Sadly
flexlibrary depends on the old version of pyyaml so I needed to fork it so we can upgrade pyyaml in this repo. I tried to submit the change upstream, but it looks like the repo is not maintained anymore - https://github.com/pipermerriam/flex.