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

Add ability to get parsed configs #3369

Open
wimglenn opened this issue Apr 4, 2018 · 12 comments
Open

Add ability to get parsed configs #3369

wimglenn opened this issue Apr 4, 2018 · 12 comments
Labels
type: enhancement new feature or API change, should be merged into features branch type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@wimglenn
Copy link
Member

wimglenn commented Apr 4, 2018

Could devs please add a high-level interface to get parsed configuration values, the ones which will eventually be used? Currently I check in request.config.option.myconfig (which comes from command-line) and if that's None then I check in request.config.getini('myconfig') (which comes from config file). I'm not sure if there can be maybe environment variables inbetween cmdline and file conf too, because I don't generally use those. It would be nice to have one config map to encapsulate this layered lookup (may I suggest a collections ChainMap - cmdline, environment, file, defaults)

...or is it already there, and I'm missing something obvious?

@pytestbot pytestbot added platform: mac mac platform-specific problem type: enhancement new feature or API change, should be merged into features branch labels Apr 4, 2018
@RonnyPfannschmidt
Copy link
Member

there is nothing like that at the moment

@RonnyPfannschmidt RonnyPfannschmidt added type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature and removed platform: mac mac platform-specific problem labels Apr 4, 2018
@pytest-dev pytest-dev deleted a comment from pytestbot Apr 5, 2018
@nicoddemus
Copy link
Member

@wimglenn are you thinking about a method in config? Any suggestions for a name?

It should be very simple to implement, not sure if it is a good idea though... your suggestion is that would be used when a plugin provides the same option both from the command-line and in pytest.ini? Nowadays this should no longer be necessary in most cases: you define only the ini option, and users can change that from the command line using the -o flag.

@wimglenn
Copy link
Member Author

wimglenn commented Apr 5, 2018

Any suggestions for a name?

Yes, type(config).__getitem__. Doesn't seem to be used for anything currently. Maybe also add config.get method to make it quack like a mapping?

@nicoddemus
Copy link
Member

Not sure we should use a special method for that, it might be not entirely obvious that config['some_option'] will conveniently obtain that option from multiple places... TBH I don't have a better solution to offer either.

(badgers badgers badgers..., snake snake oooh that's a snake!!)

@wimglenn
Copy link
Member Author

wimglenn commented Apr 6, 2018

Hmm, on the contrary, config['some_option'] providing the correct value seems the "one obvious way" to me. It should of course be documented that the lookup is layered.

I've actually incorrectly assumed the attribute access request.config.option.some_option would be providing the resolved option, and only found out the hard way that it remained None when an opt specified in file instead of by cmd line. Had to go spelunking in source code and debugger for a while before I found the existence of request.config.getini.

real life badger badger.jpg

@RonnyPfannschmidt
Copy link
Member

well - there currently is a major probem with that one - ini config items and cli option items have ever so slightly different semantics for definition at the moment

also there is the idea of eventually supporting pyproject.toml, which is different yet again

just pretending these things are the same will be problematic - i would like to see a unification that brings together the config formats and the cli in a more consistent manner myself

@feuillemorte
Copy link
Contributor

Does pytest have final applied options? I mean we can write something like

    def __getitem__(self, item):
        option = None
        try:
            option = getattr(get_config().option, item)
        except ValueError:
            pass
        return option

in Config class, but I don't see some options like -v and etc and options from ini file.

@RonnyPfannschmidt
Copy link
Member

@feuillemorte -1 - that just raises the responsibilities of the config object, currently those are broken and complex ^^

@feuillemorte
Copy link
Contributor

Ok, I want to try to understand how to unificate different config options. Could you tell me what I need to watch for this?
There are getoption and getini functions. But how I can know which option was applied if they are identical?

@RonnyPfannschmidt
Copy link
Member

@feuillemorte many of those are "deprecated" apis which where not propperly deprecated as we simply didnt deprecate

i would like to see a common way to declare parsed options, inifile components, toml values, but i dont really want to even work on that if we cant even initialize the config object without heisenbugs

@nicoddemus
Copy link
Member

It is not clear to me exactly what are the problems you are discussing @RonnyPfannschmidt (mind I'm not saying that there are none, just that I honestly don't know what you are talking about specifically).

Could you please elaborate exactly what are the current problems and what you envision would be the proper solution? Points which are fuzzy to me:

ini config items and cli option items have ever so slightly different semantics for definition at the moment

that just raises the responsibilities of the config object, currently those are broken and complex ^^

many of those are "deprecated" apis which where not propperly deprecated as we simply didnt deprecate

on that if we cant even initialize the config object without heisenbugs

And I know my way a bit around the code, people who are new to the code probably don't have any idea of what the above means.

I fear that without a proper explanation this issue will sit here effectively blocked forever because nobody else but you knows what should be done to move this forward. 😕

@RonnyPfannschmidt
Copy link
Member

a good starting point would be a new way to introduce configuration values, and then expand ini configuration and commandline configuration to push to configuration values instead of the current storage

followin gthat we could then deprecate the distinct old ways and migrate to the new system

before doing that i would like to de-tangle config object initialization, as we currently initialize config objects in a inconsistent way that's broken for xdist and certain other usages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new feature or API change, should be merged into features branch type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
Development

No branches or pull requests

5 participants