Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #2240 (closed enhancement: migrated)

Opened 5 years ago

Last modified 3 years ago

Improve the config system of TG2

Reported by: chrisz Owned by:
Priority: high Milestone: 2.2
Component: TurboGears Version: trunk
Severity: major Keywords: config
Cc:

Description

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.

Change History

comment:1 Changed 5 years ago 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.

comment:2 follow-up: ↓ 3 Changed 5 years ago 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 isn’t 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).

comment:3 in reply to: ↑ 2 ; follow-up: ↓ 6 Changed 5 years ago 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.

comment:4 Changed 5 years ago by jorge.vargas

  • Version changed from 2.0b6 to trunk
  • Milestone changed from __unclassified__ to 2.1

-1 on ConfigObj? something? 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.

comment:5 Changed 5 years ago by jorge.vargas

funny, that's a trac macro :p

What I wanted to say was

[[something]]

sorry for not hitting preview.

comment:6 in reply to: ↑ 3 Changed 5 years ago 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.

comment:7 Changed 5 years ago 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.

comment:8 Changed 4 years ago by mramm

  • Milestone changed from 2.1 to 2.2

comment:9 Changed 3 years ago by pedersen

  • Status changed from new to closed
  • Resolution set to migrated
Note: See TracTickets for help on using tickets.