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

Failed to load bare array from json file #89

Closed yujunz closed 6 years ago

yujunz commented 6 years ago

Description

Loading from json file containing bare array like [1,2] will raise exception.

How to reproduce

(py27) ➜  python-anyconfig git:(master) ✗ echo "[1,2]" > list.json
(py27) ➜  python-anyconfig git:(master) ✗ anyconfig_cli list.json
/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/requests/__init__.py:80: RequestsDependencyWarning: urllib3 (1.23) or chardet (3.0.4) doesn't match a supported version!
  RequestsDependencyWarning)
Loading: list.json
Traceback (most recent call last):
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/bin/anyconfig_cli", line 11, in <module>
    sys.exit(main())
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/cli.py", line 351, in main
    diff = _load_diff(args)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/cli.py", line 316, in _load_diff
    ac_schema=args.schema)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/api.py", line 409, in load
    **options)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/api.py", line 354, in multi_load
    ac_template=ac_template, ac_context=cnf, **opts)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/api.py", line 226, in _single_load
    return psr.load(ioi, **options)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/backend/base.py", line 274, in load
    cnf = self.load_from_path(ioi.path, container, **options)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/backend/base.py", line 463, in load_from_path
    return self.load_from_stream(inp, container, **kwargs)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/backend/base.py", line 633, in load_from_stream
    **options)
  File "/Users/yujunz/Workspace/python-anyconfig/.tox/py27/lib/python2.7/site-packages/anyconfig/backend/base.py", line 565, in load_with_fn
    return container() if ret is None else container(ret)
TypeError: cannot convert dictionary update sequence element #0 to a sequence
ssato commented 6 years ago

Hi, thanks for your report!

[1, 2] (top level array) is not a valid JSON expression originally although it became valid in the later RFCs these days. And anyconfig expects inputs are mapping objects because typical configurations consists of, maybe nested mapping (key and value) objects. In other words, it's by design that anyconfig cannot load the JSON data consists of top level array.

I don't have a plan to extend anyconfig to process such data correctly right now, though, if you have or know such use cases, please let me know details about them and I might change my mind perhaps.

yujunz commented 6 years ago

We happened to have some legacy configuration file in such format, i.e. top-level array. It is already widely used in production system. There are also some configuration in other format such as yaml and ini. So I'm thinking of utilizing python-anyconfig to provide a universal interface for configuration management. For backward compatibility, top array JSON must be supported.

What steps should I take to extend python-anyconfig?

I can see from the log message that the content is loaded correctly and it seems the issue of container that an array can not be converted to <type 'dict'>.

load_fn = <bound method Parser.load of <anyconfig.backend.json.Parser object at 0x10a6f6a10>>
content_or_strm = <closed file '/Users/yujunz/Workspace/pdm-cli/tests/fixture/pkg_ver.list.json', mode 'r' at 0x10a548ae0>
container = <type 'dict'>, options = {'object_hook': <type 'dict'>, 'object_pairs_hook': <type 'dict'>}
ret = [{'model': 'bin', 'name': 'inetagent', 'reponame': 'admin', 'version': 'v1.17.30.03.p13.cf284fa'}, {'model': 'bin', 'name': 'inetdeploy', 'reponame': 'admin', 'version': 'v1.17.30.03.p10.d83a207'}]

    def load_with_fn(load_fn, content_or_strm, container, **options):
        """
...
>       return container() if ret is None else container(ret)
E       ValueError: dictionary update sequence element #0 has length 4; 2 is required
yujunz commented 6 years ago

Another interesting finding is that it is supported by loads when giving additional options

>>> anyconfig.loads("[1,2]", ac_parser='json', ac_dict=list)
[1, 2]
>>> anyconfig.loads('[1,2]', ac_parser='json', object_hook=list)
[1, 2]
ssato commented 6 years ago

FYI.

I extended anyconfig to work with such cases in the next branch.

ssato@localhost% PYTHONPATH=. bpython3                                       ~/repos/public/github.com/ssato/python-anyconfig.git
bpython version 0.17.1 on top of Python 3.6.5 /usr/bin/python3
>>> import anyconfig
>>> cp = "/tmp/t.json"
>>> anyconfig.open(cp, mode='w').write("[1, 2]\n")
7
>>> open(cp).read()
'[1, 2]\n'
>>> c = anyconfig.load(cp)
>>> c
{'__data': [1, 2]}
>>>
ssato commented 6 years ago

FYI.

In the same branch, I changed its behavior a little. If it's OK, I'll merge it into the master branch and release the next version asap after I could make it and other changes stable.

ssato@localhost% PYTHONPATH=. bpython3      ~/repos/public/github.com/ssato/python-anyconfig.git
bpython version 0.17.1 on top of Python 3.6.5 /usr/bin/python3
>>> import anyconfig
>>> cp = "/tmp/t.json"
>>> anyconfig.open(cp, mode='w').write("[1, 2]\n")
7
>>> anyconfig.open(cp).read()
'[1, 2]\n'
>>> c = anyconfig.load(cp)
>>> c
[1, 2]
>>>
yujunz commented 6 years ago

Great.

I have a quick test locally. It seems still not working in my case.

        try:
>           return container() if ret is None else container(ret)
E           ValueError: dictionary update sequence element #0 has length 4; 2 is required

It seems only TypeError is handled but actually, a ValueError is raised.

ssato commented 6 years ago

Could you please let me know what was the input causing ValueError raised? I want to figure out it and add to test cases.

ssato commented 6 years ago

Never mind, I found out the case, nested lists causing that. Try to fix asap...

yujunz commented 6 years ago

If it's OK, I'll merge it into the master branch and release the next version asap after I could make it and other changes stable.

I tried the next branch and it works great.

Let me know if anything I can do to accelerate the release.