Project

General

Profile

Actions

Feature #9

open

Split out config to separate package?

Added by Lance Edgar over 2 years ago. Updated over 1 year ago.

Status:
In Progress
Priority:
Normal
Assignee:
Start date:
08/06/2022
Due date:
% Done:

0%

Estimated time:

Description

I've pondered this for years, and wanted to get some thoughts down. At this time I remain unconvinced that a separate package is needed, but surely some sort of refactor is in order eventually.

Below are various thoughts so far.


The RattailConfig class has served pretty well but it needs some love, esp. since it is the "most fundamental" part of Rattail. I say that because e.g. in a new Python shell, the first thing to do is usually call make_config().

Now the AppHandler class has blurred the lines a bit, since it's also (effectively) a singleton, so 1-to-1 with the global config object (got with config.get_app()). Some of the methods on RattailConfig probably belong on AppHandler instead.

Or maybe there should be some make_app() function, and the AppHandler returned should have methods to read/write config settings, instead of passing around the config object..? IDK, it seems like calling config.get() is better than app.get_setting()? (Or am I just being nostalgic?) It would also mean calls like somefunc(config) would be replaced by somefunc(app) in which case that may be "difficult" to refactor anytime soon.

Splitting out the main Config class to a separate package seems like a good idea, to make it available for non-Rattail projects. But the question is, does it actually do something useful enough for anyone else? To my mind it offers:

If I were to bother splitting it out, it should be more "modular" somehow and let config values come from other sources etc.

But today I did yet another search on PyPI, and found the python-configuration package which actually seems to do already, what I had in mind. It appears at first glance that it should be possible to add a custom Configuration for the DB settings table logic, and presumably the Beaker cache as well.

Which then begs the question, if python-configuration is so swell then why am I not just using it? (Well first of all, I've looked to see what's out there before, but today was the first time I found it; apparently came out Jan 2019.) But maybe it should get used..TBD. Probably depends on how much of the config-related things AppHandler needs to take on, which e.g. then might delegate to either a RattailConfig or Configuration to read/write settings?


Related issues 1 (1 open0 closed)

Related to Rattail - Feature #8: Add some sort of caching for config from DB settingsNewLance Edgar07/31/2022

Actions
Actions #1

Updated by Lance Edgar over 2 years ago

  • Related to Feature #8: Add some sort of caching for config from DB settings added
Actions #2

Updated by Lance Edgar over 1 year ago

  • Status changed from New to In Progress

Have added a new RattailConfiguration class, which uses the python-configuration library where applicable and should be a drop-in replacement for old class (which still remains). Also added a temporary wrapper class which delegates either to old config object, or new one. Technically the wrapper is what's being passed around in the app currently..

A notable change in the new class is that it mostly drops/ignores the concept of "sections" and "options" which came from Python's configparser module. It now is concerned only with the "key" which is essentially "section.option" the same as we've always done for the DB settings table.

# old way (but still works with new class)
config.get('foo', 'bar')

# new way (optional but works the same)
config.get('foo.bar')

Enable the new class with config like:

[rattail.config]
use_configuration = true

I've enabled this in a few apps of lower importance. Will get more aggressive with that soon, and ultimately get rid of the older RattailConfig class (and the wrapper) altogether.

This has shown me however, that I may always need a Rattail-specific config class, and can never e.g. use the one(s) from python-configuration as-is. In part that is because of the sheer number of config.get('foo', 'bar') occurrences in the wild.

Actions

Also available in: Atom PDF