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 configuration file support #77

Closed
Breakthrough opened this issue Feb 10, 2022 · 7 comments
Closed

Add configuration file support #77

Breakthrough opened this issue Feb 10, 2022 · 7 comments
Labels
Milestone

Comments

@Breakthrough
Copy link
Owner

The amount of command line options is already growing fairly large, and a number of parameters would likely be better served as configuration options. A configuration file should also allow tweaking the default behaviours, and possibly expose more options (e.g. for background subtractors).

@ocram
Copy link

ocram commented Feb 10, 2022

Can configuration files perhaps always be optional and ultimately be overriden by CLI arguments (if present)? And when they are used, the CLI output should include a line showing that a file (and which one) is in use.

It’s useful to have self-contained, concise CLI statements where you know exactly what they do, which are not dependent on some state stored somewhere on the file system, perhaps in one of multiple possible paths you have to look up in the documentation – especially if you’re new to the software or to a specific integration of it. Further, integration into larger scripts, perhaps with varying parameters, is easier if you can do anything with CLI arguments (and don’t have to write multiple temporary config files from the script).

Reasonable default values and an interface that lets you do anything from the CLI, both properties that this software currently has, also guarantees that you can learn/experiment by just executing the program and then gradually adding individual parameters to see how this affects results.

Perhaps something like .dvr-scan.json, either in the directory from where the program is invoked, or from the current user’s home directory, or from a directory given via --config-file <file>, could be used (optionally).

Some more obscure options or options with complex structures, e.g. for background subtractors, could be exceptions that go only into the configuration files, of course.

@Breakthrough
Copy link
Owner Author

Breakthrough commented Feb 10, 2022

Sorry for not elaborating more in the issue description - indeed much of what you suggest is the intention for this, so to address your main points/questions for clarification:

Can configuration files perhaps always be optional and ultimately be overriden by CLI arguments (if present)? And when they are used, the CLI output should include a line showing that a file (and which one) is in use.

Yes! Options specified in the command line will always override what any configuration file specifies. In addition to searching for one in the program/home directory, indeed I plan on adding a command line argument for you to specify a path to a specific file.

It’s useful to have self-contained, concise CLI statements where you know exactly what they do, which are not dependent on some state stored somewhere on the file system, perhaps in one of multiple possible paths you have to look up in the documentation – especially if you’re new to the software or to a specific integration of it.

Indeed, the intention is not to replace any existing command line arguments, but as you said, allow tweaking of obscure/complex options (e.g. background subtractor parameters, the colour of a bounding box), and also to override the default arguments should you want different default values each time you run DVR-Scan.

To ease finding the config file, I was planning on considering something like the appdir package which provides a nice system-agnostic interface for iterating through the various locations. I would also provide a .json file example with all options set to their defaults.

I think the best general approach for determining which parameter to use for a given option is:

  1. If value specified via command line, use that value
  2. If --config-file <file> is specified, use that value if it exists
  3. If .dvr-scan.json exists in appdir.user_config_dir('dvr-scan', False), use that value if it exists
  4. Use default value for parameter

I would like to avoid implicitly including a config from the working directory since you could easily add a bash alias to do that, and it means less surprises (there would be only one user-wide location for the configuration on every platform). I also filed a new bug to track adding more logging for debugging purposes (#79), and could then indicate the location of the config files being used, and a JSON dump of the resulting params.

Hopefully this clears your questions/concerns, but happy to hear any other feedback or suggestions you might have. Thanks for the reply!

@ocram
Copy link

ocram commented Feb 12, 2022

Sounds good!

With a dedicated parent directory like .dvr-scan, the configuration file itself could be named config.json, of course.

I think JSON is pretty common, easily processed in Python, and has fewer problems than YAML. For those that really need comments in their configuration files (as opposed to the project page, the documentation or a separate README), pre-processing or (JavaScript) minification is recommended.

@Breakthrough
Copy link
Owner Author

For now will end up probably just using configparser from Python but will consider that for the future.

I think using appdir, the resulting path would be something like ~/.config/dvr-scan/config.ini on Linux.

@Breakthrough
Copy link
Owner Author

Done in v1.5 branch, still need to provide better documentation, but would love some feedback if anyone is willing to test (you can find .whl's on the AppVeyor build artifacts at https://ci.appveyor.com/project/Breakthrough/dvr-scan/history)

@Breakthrough
Copy link
Owner Author

Breakthrough commented Jul 2, 2022

Sorry forgot to link to the template containing all config options currently implemented:

https://github.com/Breakthrough/DVR-Scan/blob/v1.5/dvr-scan.cfg

This shows the current options that are supported in v1.5. There might be a few more before release, but this covers the majority. As for the documentation, considering just including that file directly on this page in the docs.

@Breakthrough
Copy link
Owner Author

Documentation is now complete: https://dvr-scan.readthedocs.io/en/v1.5/guide/config_file/

I'll try to release a beta/release candidate for v1.5 before the final release, but the current builds are more than stable enough for deployment if anyone wants to try it early.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants