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

Bug with template OneOf() #88

Open
mozesa opened this issue May 12, 2020 · 2 comments
Open

Bug with template OneOf() #88

mozesa opened this issue May 12, 2020 · 2 comments

Comments

@mozesa
Copy link

mozesa commented May 12, 2020

Dear Adrian,

first of all I would like to say Thank you for this awesome package.
It is really great.

Maybe I was not enough thorough but 'import_write': confuse.OneOf([bool, 'ask', 'skip']), allows me to use such a values like import_write: asdfasdfasdf in config_default.yaml.

The example is the example of package so you can reproduce it by yourself.

What do I do wrongly?

Thanks for your help in advance.

@sampsyo
Copy link
Member

sampsyo commented May 12, 2020

Hmm; thanks for pointing this out. Here's a short little script to demonstrate the problem:

import confuse
config = confuse.Configuration('foo')
config.add({ 'import_write': 'qux' })
print(config['import_write'].get(confuse.OneOf(['ask', 'skip'])))

Looking at the code, I have realized that I have no idea how OneOf is supposed to work. It was introduced in 473bc036 and it has had several strange things from the beginning. The salient weirdness is this exception handler:

confuse/confuse.py

Lines 1392 to 1393 in 3bf9680

except ConfigError:
pass

I don't think we should be handling that root exception at all, but silently passing seems like it is certainly a problem. Other weird things include this mutable update when trying to access a value:

self.template = template

I don't really understand what that template argument is for.

Finally, this special handling of filenames in OneOf:

confuse/confuse.py

Lines 1374 to 1375 in 3bf9680

if isinstance(candidate, Filename) and \
candidate.relative_to:

seems pretty weird—why would these two templates need to interact in a special way?

Anyway, it seems like OneOf might need to be rethought from the ground up. (And of course we should add a test case to catch this bug!)

@mozesa mozesa changed the title Bug with template? Bug with template oneOf() May 12, 2020
@mozesa mozesa changed the title Bug with template oneOf() Bug with template OneOf() May 12, 2020
@buanzo
Copy link

buanzo commented May 17, 2020

Hi, I got to this issue while searching for better usage examples of confuse.OneOf() and templates. I was sure I was doing something wrong. I tested by changing a config value to a non-Integer, and tested using confuse.Integer(): It worked. So I am here to confirm that OneOf() is indeed not working, but I got a day or two too late.

PS: This library is awesome, btw. My project Hume will benefit a lot from it. Thanks.

buanzo added a commit to buanzo/hume that referenced this issue May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants