juju / charm-tools

Tools for charm authors and maintainers
Other
42 stars 64 forks source link

ruamel.yaml 0.18 hard deprecated old PyYAML functions including load, safe_load, etc (scan, parse, compose, load, emit, serialize, dump and their variants _all, safe_, round_trip_, etc) #668

Closed lathiat closed 10 months ago

lathiat commented 10 months ago

My older stable charm builds using charm-tools 2.8.x broke today due to the release of ruamel.yaml 0.18

As noted here https://pypi.org/project/ruamel.yaml/

As announced, in 0.18.0, the old PyYAML functions have been deprecated. (scan, parse, compose, load, emit, serialize, dump and their variants (all, safe, roundtrip, etc)). If you only read this after your program has stopped working: I am sorry to hear that, but that also means you, or the person developing your program, has not tested with warnings on (which is the recommendation in PEP 565, and e.g. defaultin when using pytest).

In the OpenStack charm builds particularly, the error doesn't actually surface for some reason. The charm-build process just exits with an error code but no message. I had to use python -m trace to find the code throwing the error.

We use safe_load in many places but the place it errors out for me in particular is from charmtools/build/config.py:57 BuildConfig.configure()

We'll likely need to pin to ruamel.yaml<0.18.0 in the short term, for both 2.8.x and master and work on supporting the newer version.

lathiat commented 10 months ago

Looks like master is already pinned to ruamel.yaml < 0.18 "so that dependencies can be consumed from versions from the Ubuntu distribution wherever possible.". So, for now, we just need to make the same change in the 2.8 branch.

lathiat commented 10 months ago

For some reason I can't figure out, the warnings.warn() that the ruamel library calls before running sys.exit(1) is not logged by charm-build. So this error is entirely not obvious, I could only figure it out by adding "python -m trace --trace" to the charm-build invocation. That will make it hard for impacted charms to debug this issue (which is a lot of openstack stable charms). They all pin charm-tools to specific 2.8.x versions so even once we commit a fix people are likely to be confused by the lack of error in th elog.

logging.captureWarnings(True) is run before the config code runs and if I add a call to warnings.warn from inside build/config.py right before the yaml call, it's logged, but once it calls into the ruamel library it is not. I must be missing something to do with scoping or when/the order the code is loaded here.

However I think sys.exit(1) is not really ideal behaviour for a library, so I filed an upstream bug to ask them to consider something that may be more obvious if warnings are disabled such as raising an exception or simply removing the function definition (which would then raise an exception): https://sourceforge.net/p/ruamel-yaml/tickets/486/

lathiat commented 10 months ago

Fixed by #669 and the subsequent release of 2.8.8