search

ssato / python-anyconfig

Python library provides common APIs to load and dump configuration files in various formats
MIT License
277 stars 31 forks source link

change: don't use ruamel.yaml if it is too old #97

Closed masatake closed 5 years ago

masatake commented 5 years ago

ruamel_yaml uses YAML class. However, an older version of ruamel.yaml doesn't define YAML class.

https://bitbucket.org/ruamel/yaml

0.15.0 (2017-06-04):

    ...
    added YAML class for new API

If an older version is installed on a system, python-anyconfig doesn't work at all.

This change adds code verifying the existence of YAML class.

With this change, python-anyconfig chooses pyyaml as fallback yaml parser on Fedora 28, where only version 0.13 of ruamel-yaml is available.

Signed-off-by: Masatake YAMATO yamato@redhat.com

ssato commented 5 years ago

Thanks for your report and PR! However, it broke all tests and it needs to be fixed before merge.

ssato commented 5 years ago

Related: #98

ssato commented 5 years ago

Maybe there are two choices.

# put this block just before global variable defs.
try:
    ryaml.YAML  # noqa
except AttributeError:
    ImportError('ruamel.yaml may be too old; no YAML class is defined')
masatake commented 5 years ago

I don't know python well. However, I think reporting ImportError is better. anyconfig/backend/yaml/__init__.py expects no IMPORT error.

The AttributeError is an internal error. The ImportError is a part of interface between anyconfig/backend/yaml/__init__.py and ` anyconfig/backend/yaml/ruamel_yaml.py.

ruamel_yaml.py has a responsibility to report whether the backend implementation is usable or not in IMPORT error. My fix is for making ruamel_yaml.py fulfils the responsibility.

ssato commented 5 years ago

I guess that the original problem of this PR should be resolved by the commit c429c62. Could you please test the later one after that commit in the next branch?

masatake commented 5 years ago

Sorry, I have no test environment. I'm using a version of python-anyconfig distributed as a rpm package. When making a patch, I modify the installed version of code directly.

Instead of testing the latest code locally, I will make a pull request that tests the latest code on Fedora 28 environment on circle ci. Fedora 28 provides only older ruamel.yaml.