Skip to content
This repository was archived by the owner on Apr 30, 2021. It is now read-only.

Pin PyYAML==3.13 and fix unit tests#37

Merged
an2deg merged 4 commits into
an2deg:masterfrom
mzagozen:pyyaml-3.13
Apr 30, 2019
Merged

Pin PyYAML==3.13 and fix unit tests#37
an2deg merged 4 commits into
an2deg:masterfrom
mzagozen:pyyaml-3.13

Conversation

@mzagozen
Copy link
Copy Markdown
Contributor

There was a new version of PyYAML 5.1 released in March: https://lists.gt.net/python/python/1413898.

This package requires PyYAML>=3.10, so any new installations will pull the new version. But, the new version has a backwards incompatible change to the yaml.load() function. See https://msg.pyyaml.org/load.

The change breaks the !include tag parsing for this package. The functionality in PyYAML is still evolving and will change for the better in 5.2 (see yaml/pyyaml#279). Thus I would rather wait with >=5.1 compatibility fixes until 5.2.

In this branch, I pinned PyYAML to last working 3.13 working version. Additionally, I fixed the unit tests that were failing.

@an2deg please activate the project in Travis: https://travis-ci.org/an2deg/pyraml-parser. Only the repo admin can do that.

PyYAML>=5.1 introduces a backwards-incompatible change with regards to
the YAML parser. Until this package is made compatible, just stick with
the old version. See https://msg.pyyaml.org/load
The parser outputs OrderedDict instances. Tests were not fixed after
this change :o
Copy link
Copy Markdown
Owner

@an2deg an2deg left a comment

Choose a reason for hiding this comment

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

Looks good for me. I'm gonna merge it

@an2deg an2deg merged commit 2d8d230 into an2deg:master Apr 30, 2019
@mzagozen mzagozen deleted the pyyaml-3.13 branch May 6, 2019 12:22
@aonamrata
Copy link
Copy Markdown

Hi @mzagozen, @an2deg Do you have any plans to update to latest pyyaml? We started getting this warning on github with older pyyaml version https://nvd.nist.gov/vuln/detail/CVE-2017-18342

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants