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

Watcher include directive #1123

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

biozz
Copy link
Contributor

@biozz biozz commented Mar 14, 2020

This can be considered as proof of concept for #1119 and it is still "work in progress".

I added support for include directive in watcher section, which is compatible across currently supported versions.

This is an attempt to implement circus-tent#1119. For now only configs and tests
are added and some drafts in the config loader. This is still work in
progress.
- rename config in tests
- rename test case
- add setting of config option from include
@coveralls
Copy link

coveralls commented Mar 14, 2020

Coverage Status

Coverage increased (+0.09%) to 63.053% when pulling 28ba922 on biozz:wip-1119-watcher-include into 422d64d on circus-tent:master.

- Refactor read_config method with two new local methods

  This was necessary to avoid code duplication, because include directive
  in `[watcher]` is basically the same as in `[circus]`. This allowed the reuse of `_scan` method. I also added underscores to
  arguments names, because they were shadowed.

  - `_init_config` - takes config path, has a parser option with default value
    and return parsed config from the path
  - `_get_includes` - takes config object and section name and returns a
    list of paths to read options from

- Add a required section name for watcher includes to avoid usage of a
  fake `[tmp]` section from previous commits
  according to [documentation](https://docs.python.org/3/library/configparser.html)
  sections are required and in my opinion should be used going forward

-  Expand test case to ensure included configs can be loaded from folders
   and using wildcards
@biozz
Copy link
Contributor Author

biozz commented Mar 17, 2020

In the latest commit I introduced a new section header [included] in included watcher configs, because according to docs section names are required and my previous attempt to use [tmp] was kinda looking bad.

There was also a minor refactoring happening inside read_config method.

I haven't updated documentation just yet, but this should be easy, given the options are basically the same.

I would appreciate any feedback on this PR.

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

Successfully merging this pull request may close these issues.

2 participants