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 support for include directive in watchers #1119

Open
biozz opened this issue Mar 5, 2020 · 3 comments
Open

Add support for include directive in watchers #1119

biozz opened this issue Mar 5, 2020 · 3 comments
Assignees
Labels

Comments

@biozz
Copy link
Contributor

biozz commented Mar 5, 2020

There is a include directive in circus section of the config - https://circus.readthedocs.io/en/latest/for-ops/configuration/#circus-single-section

But there is no such thing in watcher section.

It would be nice to have it there, because there are a lot of cases when you define the same config for different watchers and only watcher id is changing.

For example - https://circus.readthedocs.io/en/latest/for-ops/papa/ has at lease these duplicated rows lines:

stdout_stream.class = FileStream
stdout_stream.filename = /var/logs/logger.log
stdout_stream.max_bytes = 10000000
stdout_stream.backup_count = 10
stderr_stream.class = FileStream
stderr_stream.filename = /var/logs/logger.log
stderr_stream.max_bytes = 1000000
stderr_stream.backup_count = 10

So if we have include directive for this we can do something like this:

; circus.ini
[circus]
loglevel = info

[watcher:logger]
cmd = /my_service/env/bin/python logger.py run
working_dir = /my_service
graceful_timeout = 10
singleton = true
stop_signal = INT
include = logs.ini

[watcher:web_app]
cmd = /my_service/env/bin/uwsgi --ini uwsgi-live.ini --socket fd://$(circus.sockets.web) --stats 127.0.0.1:809$(circus.wid)
working_dir = /my_service/web_app
graceful_timeout=10
stop_signal = QUIT
use_sockets = True
include = logs.ini

; logs.ini
stdout_stream.class = FileStream
stdout_stream.filename = /var/logs/$(circus.wid).log
stdout_stream.max_bytes = 10000000
stdout_stream.backup_count = 10
stderr_stream.class = FileStream
stderr_stream.filename = /var/logs/$(circus.wid).log
stderr_stream.max_bytes = 1000000
stderr_stream.backup_count = 10
@k4nar
Copy link
Contributor

k4nar commented Mar 6, 2020

I like the idea, although I think that keeping everything in the same file would be a great plus. Maybe we could introduce a new section in the conf, and allow to include it?

@biozz
Copy link
Contributor Author

biozz commented Mar 7, 2020

If you use include syntax you are implying that it should work the same way in circus and other sections, making it consistent. But I agree you can be lost in your config files at times.

Having a new kind of section with extra configs, on the other hand, would work only for watcher. Because you usually don't have repeating stuff in socket or env. And I am also not sure how can we call that new section kind. Something like common or extra. Calling it include will make it inconsistent in my opinion.

And this looks a bit strange to me:

[watcher:app_1]
cmd = echo
extra = logs

[watcher:app_2]
cmd = echo
extra = logs

[extra:logs]
stdout_stream.class = FileStream

@k4nar, could you please give an example config how you see it done?

@k4nar
Copy link
Contributor

k4nar commented Mar 13, 2020

I don't really have a better solution to offer :) . Let's implement the include instead, as it's probably the easiest & less confusing option.

biozz added a commit to biozz/circus that referenced this issue Mar 14, 2020
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.
@biozz biozz self-assigned this Aug 6, 2020
@biozz biozz added the feature label Aug 6, 2020
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