Improve the config system of TG2 #13

Open
pedersen opened this Issue Sep 24, 2012 · 4 comments

Projects

None yet

2 participants

@pedersen
TurboGears member

This issue existed in Trac. The original can be viewed at http://trac.turbogears.org/ticket/2240

This issue existed on SourceForge. The original can be viewed at https://sourceforge.net/p/turbogears2/tickets/72

@pedersen
TurboGears member

Original Author: pedersen, Original Timestamp: 2011-03-29 02:48:31.570000

Original Body: The current configuration system of TG2 is somewhat confusing and error-prone.

Actually we have two configuration systems which in my view are both problematic: The *.ini files and the *_cfg.py files.

1) The problem with the *.ini files is that - unlike TG1 - the settings are not evaluated. So if you set my.setting = false, your setting will be actually the string 'false', not the boolean value False. Same problem for ints, lists or other Python objects. So you need to evaluate the config settings with functions like asbool() which is inconvenient, not performant and you can easily forget to do this.

2) The problem with the *._cfg.py is that you have to write base_config['my.setting'] = False instead of simply my.setting = false which is much clearer and easier to write if you have many settings. Also, persons who need to customize application specific settings may not always be Python coders and can easily mess up things in the *.py files.

But the main problem is that both syntax and semantics of the 2 config systems differ. I.e. if you move a setting from app_cfg.py to deployment.ini, then you need to change the notation and you need to change the way how you access the setting in your application in case it is not a string.

Suggested solution:

1) Use ConfigObj instead of ConfigParser for the *.ini files (maybe also rename them to *.cfg to make this change more obvious). Alternatively, use ConfigParser, but evaluate the settings immediately after reading the config file. This is also how it is done by CherryPy. This way the TG2 config system would also become more compatible with TG1 again.

2) Maybe add a file base_cfg.py that gets imported as base_config in app_cfg.py. Then you can simply set setting = False in base_cfg.py instead of base_config.setting = False in app_cfg.py.

3) Alternatively or in addition to base_cfg.py, there should be a normal app.ini resp. app.cfg file. In my projects I have many settings that are application specific, but need to be customized by the person who is deploying the application (who may not be a Python coder), not by an application developer.

02/26/09 12:29:23 changed by Gustavo

I'm -10 on this: I think switching to PasteDeploy config files was a wise move because it makes things much more extensible, with its application factories, filters, etc. Also, what about "paster serve"? We'll have to create our own PasteScript replacement just to use a different, incompatible config file format.

Also, I can't see anything stopping you from doing what you want to do: Just define an application-specific configuration file in XML, ConfigObj file, or whatever format you want. Then in development.ini or deployment.ini, add:

[app:main]

(...)

my_app_config = %(here)s/foobar.cfg

Finally, from {yourapp}.config.middleware:make_application, do:

parsed_config = parse_it(app_conf['my_app_config'])

This way you'd end up with the standard settings in development/deployment.ini, while your application-specific settings will be in the format you like (in settings.xml, settings.ini or settings.cfg, for example).

We may even add this functionality by default, because I think it's pretty common and more convenient.

Finally, I agree that we have to move some settings out of app_cfg.py, like those related to authentication, identification and authorization.
Delete (follow-up: 3 ) 02/26/09 15:04:54 changed by chrisz

type changed from defect to enhancement.

Right, "paster serve" assumes Paste Deploy config format, which means unevaluated strings only - so this cannot be changed so easily. But it would still make sense, as you suggested, to keep the application specific config in a different file where values are evaluated. We could keep the different file extensions *.ini and *.cfg to distinguish files which are not evaluated (i.e. you can write strings without quotes) from files where values are evaluated (i.e. you need to quote strings).

And maybe we can suggest that the Paste Deploy format gets changed in the long run. This would even simplify Paste itself - if you look at the function server_runner in paste.httpserver you will understand what I mean.

In fact, it seems that Ian recognized the problem, too (see here): "Should objects be Python-syntax, instead of always strings? Lots of code isnt usable with Python strings without a thin wrapper to translate objects into their proper types." Probably we should discuss this on the Paste mailing list first, to find out how likely this will be changed in Paste.

If it's not gonna be changed then we should consider the split into *.ini files used by paste and *.cfg used for tg app configuration or we should at least add some convenience methods to the config object which allow getting values interpreted as bool or ints (the default config parser actually has methods like getboolean() or getint(), we should then make these available as well).
Delete (in reply to: 2 ; follow-up: 6 ) 02/26/09 17:03:57 changed by Gustavo

Replying to chrisz:

If it's not gonna be changed then we should consider the split into *.ini files used by paste and *.cfg used for tg app configuration or we should at least add some convenience methods to the config object which allow getting values interpreted as bool or ints (the default config parser actually has methods like getboolean() or getint(), we should then make these available as well).

Unfortunately, I have no hope of that being changed in PasteScript in the medium-term. I mean, they're already aware of this issue and it's heavily used program... I think that if they had intentions to fix it, right now we weren't discussing this.

Therefore I think we should just provide an application-specific ConfigObj file, parsed by default while loading the middleware, while keeping a PasteScript config file for standard settings (those used by PasteScript). The drawback of this solution is to have "yet another file" in the quickstart template.
Delete 02/26/09 21:28:14 changed by jorge.vargas

version changed from 2.0b6 to trunk.
milestone changed from __unclassified__ to 2.1.

-1 on ConfigObj?
Error: Failed to load processor something

No macro or processor named 'something' found

sections are very ugly IMO and go beyond the "basic non-coder deployment" approach if we have files make then plain old .ini files.

+1 on making boolean and number values evaluate to their proper python types instead of strings, this is something we must fix on the documentation

+1 in making app_cfg.py less verbose.
Delete 02/26/09 21:33:53 changed by jorge.vargas

funny, that's a trac macro :p

What I wanted to say was

[[something]]

sorry for not hitting preview.
Delete (in reply to: 3 ) 02/27/09 11:11:01 changed by chrisz

Replying to Gustavo:

Unfortunately, I have no hope of that being changed in PasteScript in the medium-term. I mean, they're already aware of this issue and it's heavily used program... I think that if they had intentions to fix it, right now we weren't discussing this.

Anyway, I have just asked on the Paste ML. You never know, maybe they wanted to change this all the time anyway? Well probably not, but then maybe the Paste config object could at least grow some methods for getting bools and ints, like they are provided by the standard ConfigParser (as far as I understand, the TG config is stacked on the Pylons config which is stacked on the Paste config, so it should be added on the lowest level).

What's making the whole stuff even more confusing is that some values are in fact evaluated by Paste or other code and these values are stored back in the config (by code such as server_running in paste.httpserver). So you never know whether some part of the system has already evaluated a config setting when it was accessing the value before you, or whether you are still required to evaluate this thing yourself. This is a big mess.
Delete 03/05/09 04:15:42 changed by chrisz

The discussion on the Paste ML is here if anyone cares. Seems this will not be changed in Paste, but it may give us some ideas for solving this mess.

@pedersen
TurboGears member

Original Author: pedersen, Original Timestamp: 2012-08-24 01:43:03.034000

Original Body: - version: 2.1.0 --> 2.1.5

  • milestone: 2.2.0 --> 2.3.0
@amol-
TurboGears member

I have been working on this on workshop branch for TG2.4, the core idea is to provide a Configurator object where one or multiple configuration steps (features) can be registered and depend on other features.

Each feature must declare a namespace for the options, the option values converters and default values, so that options are always converted (independently from where they came from: .ini, app_cfg or paramters to "gearbox serve") and have a default value when not specified (unlike now where code is full of dict.get() calls due to possibly missing options).

This should also solve another major issue in AppConfig which is its complexity, the new Configurator is just mostly a container and configuration process is demanded to each single feature so that configuration steps are short, separated and unrelated from the configurator itself.

An experimental version (which is just able to create an "Hello World" TurboGears app) is available at:
184e657

I expect also to be able to reimplement AppConfig over the Configurator for backward compatibility so that old apps don't require major changes to the code to work on TG2.4

@amol- amol- modified the milestone: 2.3.0, 2.4.0 May 21, 2015
@amol-
TurboGears member

Since 2.3.8 there is now guarantee that all options are properly read/write from conf currently being constructer nstead of mixing AppConfig/tg.config/currently under build conf

2.4.0 will aim at allowing to add/remove configuration steps while so far they have been hardcoded in AppConfig.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment