Skip to content
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

no error raised about malformed yml #3501

Closed
adamjakab opened this issue Feb 24, 2020 · 3 comments
Closed

no error raised about malformed yml #3501

adamjakab opened this issue Feb 24, 2020 · 3 comments
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature."

Comments

@adamjakab
Copy link
Contributor

adamjakab commented Feb 24, 2020

It seems like that the loading of malformed yml files included in the configuration through the ìnclude`key does not raise the expected error.

Problem

Whilst working on PR #3498 I purposefully included a malformed yml file just to test if the newly introduced warning would also catch the error about it:

include: 
    - badformat.yml

and with the 'badformat.yml' file:

this
    will
        not
            pass
        as
    ok
yml

I expected a confuse.ConfigReadError but no errors were raised.
This is what I got:

$ ./beet
configuration error: musicbrainz.host not found

More investigation is needed.

Setup

  • OS: macos 10.13.6 (17G11023)
  • Python version: 3.7.6
  • beets version: 1.4.9
  • Turning off plugins made problem go away (yes/no): no
  • My configuration (output of beet config) is: malformed :):):)
@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Feb 24, 2020
@adamjakab
Copy link
Contributor Author

I have something new.
If I change the content of the above (badformat.yml) configuration file to this:

1

...yes just a number, I get a lovely backtrace:

(beetsdevel) Jakabimac:Beets jackisback$ ./beet config
Traceback (most recent call last):
  File "./beet", line 23, in <module>
    beets.ui.main()
  File "/Users/jackisback/Documents/Projects/Python/Beets/beets/ui/__init__.py", line 1267, in main
    _raw_main(args)
  File "/Users/jackisback/Documents/Projects/Python/Beets/beets/ui/__init__.py", line 1250, in _raw_main
    subcommands, plugins, lib = _setup(options, lib)
  File "/Users/jackisback/Documents/Projects/Python/Beets/beets/ui/__init__.py", line 1132, in _setup
    mb.configure()
  File "/Users/jackisback/Documents/Projects/Python/Beets/beets/autotag/mb.py", line 83, in configure
    hostname = config['musicbrainz']['host'].as_str()
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 517, in as_str
    return self.get(String())
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 480, in get
    return as_template(template).value(self, template)
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 1120, in value
    if view.exists():
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 207, in exists
    self.first()
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 199, in first
    return iter_first(pairs)
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 68, in iter_first
    return next(it)
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 611, in resolve
    for collection, source in self.parent.resolve():
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 611, in resolve
    for collection, source in self.parent.resolve():
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 1056, in resolve
    self.read()
  File "/Users/jackisback/Documents/Projects/Python/Beets/beets/__init__.py", line 34, in read
    self.set_file(view.as_filename())
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 993, in set_file
    self.set(ConfigSource(load_yaml(filename), filename))
  File "/Users/jackisback/opt/miniconda3/envs/beetsdevel/lib/python3.7/site-packages/confuse.py", line 142, in __init__
    super(ConfigSource, self).__init__(value)
TypeError: 'int' object is not iterable

At least now I can see where the above error message configuration error: musicbrainz.host not found comes from and how we got there.

@adamjakab
Copy link
Contributor Author

To my understanding there is no fault neither in beets nor in confuse. The yaml package will parse the input stream and serialize it but will not validate it. In case of input such as:

bad

or

123

it will NOT throw the expected yaml.error.YAMLError error that we catch in load_yaml() (confuse.py:776) in confuse.

With something a bit more elaborated like this:

aaaaaaa
x: 'aaa'
asasdasdas

it will.

So, I guess, If we want to make sure that we catch ALL malformed yml files then we probably need a yml schema validator package. I am not sure it's worth the effort.

@sampsyo
Copy link
Member

sampsyo commented Mar 2, 2020

Indeed; seems about right to me. Thanks for investigating!

@sampsyo sampsyo closed this as completed Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature."
Projects
None yet
Development

No branches or pull requests

2 participants