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 option to disable built-in log management #161

Merged
merged 2 commits into from Oct 16, 2012

Conversation

bernardd
Copy link

@bernardd bernardd commented Oct 8, 2012

This change adds the option to specify 'none' for log_dir config value. The effect is to disable the build-in log management in ChicagoBoss, allowing your to use in your own error_logger based logging system instead.

This is helpful for incorporating a CB app into a larger system where there is already an existing logging framework (handling log rotation etc) based on error_logger.

This change adds the option to specify 'none' for log_dir config value.
The effect is to disable the build-in log management in ChicagoBoss,
allowing your to drop in your own error_logger based logging system
instead.
@evanmiller
Copy link
Contributor

I wonder if "no logging" should be the default, and specifying a log_dir enables it. Or maybe have a separate log_enable option. Anyway I support the idea but I think that log_dir shouldn't be overloaded with 'none'. What do you think?

@bernardd
Copy link
Author

I don't really have a strong opinion. I went the way I did to avoid changing the default behaviour and to avoid adding a whole new config item for such a relatively trivial change, but if you think it's better done a different way I certainly won't object :)

@evanmiller
Copy link
Contributor

Let's go with "log_enable" that defaults to true. I generally like to optimize for the person looking for a specific bit of functionality, rather than overload options. And of course, the size of the change should have nothing to do with whether there should be a "whole new config item" :)

Anyway, if you send me an update I will get it merged in.

This option allows the generation of log files to be disabled.
@bernardd
Copy link
Author

Changed as requested.

@evanmiller
Copy link
Contributor

Thanks!

evanmiller added a commit that referenced this pull request Oct 16, 2012
Add option to disable built-in log management
@evanmiller evanmiller merged commit 865e662 into ChicagoBoss:master Oct 16, 2012
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.

None yet

2 participants