Conversation
juga0
force-pushed
the
feature/config
branch
2 times, most recently
from
ff90b33 to
553d5bb
Compare
meejah
left a comment
meejah
left a comment
There was a problem hiding this comment.
I think there's a nicer way with Click to do the "defaults from config file" (see inline comment). Otherwise LGTM!
bwscanner/configutil.py
Outdated
| def copy_config(cfg_path, cfg_default_path=None): | ||
| if cfg_default_path is None: | ||
| cfg_default_path = os.path.join(os.path.dirname(os.path.dirname( | ||
| os.path.abspath(__file__))), 'data', 'config.ini') |
There was a problem hiding this comment.
For this sort of thing, it's sometimes nicer to use pkg_resources (so that it still works if you've packaged a wheel or zip). See http://peak.telecommunity.com/DevCenter/PythonEggs#accessing-package-resources -- and also this interacts with setup.py, detailed here https://packaging.python.org/tutorials/distributing-packages/#package-data
| @click.group() | ||
| # FIXME: change all options to take defaults from CTX, ie config file? | ||
| @click.group(context_settings=CTX) | ||
| @click.option('--data-dir', type=click.Path(), |
There was a problem hiding this comment.
So, instead of this and the CTX.get() stuff below, I think you can: re-define the entry-point as say start and make that a method like:
def start():
config = read_config(os.path.join(DATA_DIR, CONFIG_FILE))
return cli(default_map=config)
See http://click.pocoo.org/5/commands/#overriding-defaults too
There was a problem hiding this comment.
Hmm, by reading the docs is not clear to me where start is supposed to be call.
By the "stuff below" you mean all the default= in the options?
There was a problem hiding this comment.
You meant something like juga0@388e824#diff-091411b9c979317818f6582241ab8284?
It doesn't seem simpler/nicer.
There was a problem hiding this comment.
Yeah, that's what I meant. I haven't actually done this config-file-based defaults myself, just basing it off Click's docs. I wouldn't expect you'd have to do that "for k, v in ...default_map" thing either based on what Click says but .... I can play with it a bit myself later.
juga0
force-pushed
the
feature/config
branch
2 times, most recently
from
342d088 to
e9642bb
Compare
instead of using __file__ as it would fail when packed in a zip. Include the config file in the package adding it to MANIFEST.in
* Create start function to call cli with parsed config * Parse bw_files section * Convert parsed config values to their type
|
Tests pass but not ready to merge, cli fails |
3 participants
The intention here is to read default configuration variables from a file and overwrite them when they are pass as command line arguments. I'm not sure how to integrate ConfigParser in a nice way click default_map and pass_context .