You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexandra Sava <al...@gmail.com> on 2014/04/29 18:05:52 UTC
Review Request 20850: Customize the configuration of logging level
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/
-----------------------------------------------------------
Review request for mesos.
Repository: mesos-git
Description
-------
Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
Update the documentation for 'logging_level' flag, with the values it can take.
Diffs
-----
src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
Diff: https://reviews.apache.org/r/20850/diff/
Testing
-------
Thanks,
Alexandra Sava
Re: Review Request 20850: Customize the configuration of logging level
Posted by Benjamin Mahler <bm...@twitter.com>.
Sorry for the delay, I've been bogged down quite a bit lately but I will take a look today!
Sent from my iPhone
> On May 7, 2014, at 5:17 AM, Alexandra Sava <al...@gmail.com> wrote:
>
> Hi guys,
>
> Any feedback for my changes would be welcomed.
>
>
>
> Thanks,
> Alexandra
>
>
>> On 1 May 2014 14:16, Alexandra Sava <al...@gmail.com> wrote:
>>
>> This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20850/
>>
>> On April 29th, 2014, 7:18 p.m. UTC, Ben Mahler wrote:
>>
>> src/master/master.cpp (Diff revision 1)
>> void Master::initialize()
>> 524
>> // No need to access FATAL log file; if the program
>> 524
>> google::LogSeverity logging_level = logging::getLogSeverity(flags.logging_level);
>> 525
>> // is still running, there definitely haven't been any
>> 525
>> if (flags.log_dir.isSome()) {
>> 526
>> // FATAL logs yet; a FATAL log will cause the program to crash.
>> 526
>> Try<string> log = logging::getLogFile(logging_level);
>> What about having getLogFile take a logging::Flags object? Since it already returns a Try we could even return Error() instead of EXIT(1) when the level is invalid.
>>
>> If we did this change, then logging::initialize could EXIT(1) when the level is invalid. It seems a bit unfortunate to have getLogSeverity call EXIT(1) when the input is invalid since it is exposed in the header and can be called from anywhere!
>>
>> E.g.:
>>
>> initialize(...)
>> {
>> ...
>>
>> if (flags.logging_level != "INFO" &&
>> flags.logging_level != "WARNING" &&
>> flags.logging_level != "ERROR") {
>> EXIT(1) << ...;
>> }
>>
>> // We can hide getLogSeverity from the header and we can do an internal CHECK inside, instead of EXIT(1).
>> LogSeverity severity = getLogSeverity(flags.logging_level);
>>
>> LOG_AT_LEVEL(severity) << "Logging level " + flags.logging_level + " initialized";
>>
>> ...
>> }
>> I don't really see the purpose in changing the argument of getLogFile into logging::FLags type. getLogFile returns the log file associate to the given severity, so it makes sence to provide a LogSeverity type as an argument. Also, getLogFile already does what you said: it returns an Error when the level is invalid (it never returns an Exit).
>>
>> Yes I also think it is not ok to have an EXIT in getLogSeverity as long as it is exposed in the header file. I changed that part as you suggested.
>>
>> - Alexandra
>>
>>
>> On May 1st, 2014, 11:09 a.m. UTC, Alexandra Sava wrote:
>>
>> Review request for mesos.
>> By Alexandra Sava.
>> Updated May 1, 2014, 11:09 a.m.
>>
>> Repository: mesos-git
>> Description
>>
>> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>>
>> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>>
>> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>>
>> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>>
>> Update the documentation for 'logging_level' flag, with the values it can take.
>> Diffs
>>
>> src/logging/flags.hpp (457feeeb7ca7ececf3c4e69189800ecb370053e6)
>> src/logging/logging.hpp (39c2934f733e5c058f62b5497f31f7a7057bb4c7)
>> src/logging/logging.cpp (176e49a6a2aef13be44ff910144c7321c942345a)
>> src/master/master.cpp (f205dca43f10697862e3fd3f435f1127a9d0aecb)
>> src/slave/slave.cpp (cb80609ba421b3b9a4664e600f0e53ecab8574c4)
>> src/webui/master/static/js/controllers.js (4b8487e0c285f892ad352993c81637f38df1429f)
>> View Diff
>>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Alexandra Sava <al...@gmail.com>.
Hi guys,
Any feedback for my changes would be welcomed.
Thanks,
Alexandra
On 1 May 2014 14:16, Alexandra Sava <al...@gmail.com> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
>
> On April 29th, 2014, 7:18 p.m. UTC, *Ben Mahler* wrote:
>
> src/master/master.cpp<https://reviews.apache.org/r/20850/diff/1/?file=570723#file570723line524> (Diff
> revision 1)
>
> void Master::initialize()
>
> 524
>
> // No need to access FATAL log file; if the program
>
> 524
>
> google::LogSeverity logging_level = logging::getLogSeverity(flags.logging_level);
>
> 525
>
> // is still running, there definitely haven't been any
>
> 525
>
> if (flags.log_dir.isSome()) {
>
> 526
>
> // FATAL logs yet; a FATAL log will cause the program to crash.
>
> 526
>
> Try<string> log = logging::getLogFile(logging_level);
>
> What about having getLogFile take a logging::Flags object? Since it already returns a Try we could even return Error() instead of EXIT(1) when the level is invalid.
>
> If we did this change, then logging::initialize could EXIT(1) when the level is invalid. It seems a bit unfortunate to have getLogSeverity call EXIT(1) when the input is invalid since it is exposed in the header and can be called from anywhere!
>
> E.g.:
>
> initialize(...)
> {
> ...
>
> if (flags.logging_level != "INFO" &&
> flags.logging_level != "WARNING" &&
> flags.logging_level != "ERROR") {
> EXIT(1) << ...;
> }
>
> // We can hide getLogSeverity from the header and we can do an internal CHECK inside, instead of EXIT(1).
> LogSeverity severity = getLogSeverity(flags.logging_level);
>
> LOG_AT_LEVEL(severity) << "Logging level " + flags.logging_level + " initialized";
>
> ...
> }
>
> I don't really see the purpose in changing the argument of getLogFile into logging::FLags type. getLogFile returns the log file associate to the given severity, so it makes sence to provide a LogSeverity type as an argument. Also, getLogFile already does what you said: it returns an Error when the level is invalid (it never returns an Exit).
>
> Yes I also think it is not ok to have an EXIT in getLogSeverity as long as it is exposed in the header file. I changed that part as you suggested.
>
>
> - Alexandra
>
> On May 1st, 2014, 11:09 a.m. UTC, Alexandra Sava wrote:
> Review request for mesos.
> By Alexandra Sava.
>
> *Updated May 1, 2014, 11:09 a.m.*
> *Repository: * mesos-git
> Description
>
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>
> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>
> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>
> Update the documentation for 'logging_level' flag, with the values it can take.
>
> Diffs
>
> - src/logging/flags.hpp (457feeeb7ca7ececf3c4e69189800ecb370053e6)
> - src/logging/logging.hpp (39c2934f733e5c058f62b5497f31f7a7057bb4c7)
> - src/logging/logging.cpp (176e49a6a2aef13be44ff910144c7321c942345a)
> - src/master/master.cpp (f205dca43f10697862e3fd3f435f1127a9d0aecb)
> - src/slave/slave.cpp (cb80609ba421b3b9a4664e600f0e53ecab8574c4)
> - src/webui/master/static/js/controllers.js
> (4b8487e0c285f892ad352993c81637f38df1429f)
>
> View Diff <https://reviews.apache.org/r/20850/diff/>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Alexandra Sava <al...@gmail.com>.
> On April 29, 2014, 7:18 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 524-526
> > <https://reviews.apache.org/r/20850/diff/1/?file=570723#file570723line524>
> >
> > What about having getLogFile take a logging::Flags object? Since it already returns a Try we could even return Error() instead of EXIT(1) when the level is invalid.
> >
> > If we did this change, then logging::initialize could EXIT(1) when the level is invalid. It seems a bit unfortunate to have getLogSeverity call EXIT(1) when the input is invalid since it is exposed in the header and can be called from anywhere!
> >
> > E.g.:
> >
> > initialize(...)
> > {
> > ...
> >
> > if (flags.logging_level != "INFO" &&
> > flags.logging_level != "WARNING" &&
> > flags.logging_level != "ERROR") {
> > EXIT(1) << ...;
> > }
> >
> > // We can hide getLogSeverity from the header and we can do an internal CHECK inside, instead of EXIT(1).
> > LogSeverity severity = getLogSeverity(flags.logging_level);
> >
> > LOG_AT_LEVEL(severity) << "Logging level " + flags.logging_level + " initialized";
> >
> > ...
> > }
I don't really see the purpose in changing the argument of getLogFile into logging::FLags type. getLogFile returns the log file associate to the given severity, so it makes sence to provide a LogSeverity type as an argument. Also, getLogFile already does what you said: it returns an Error when the level is invalid (it never returns an Exit).
Yes I also think it is not ok to have an EXIT in getLogSeverity as long as it is exposed in the header file. I changed that part as you suggested.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/#review41757
-----------------------------------------------------------
On May 1, 2014, 11:09 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 11:09 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>
> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>
> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>
> Update the documentation for 'logging_level' flag, with the values it can take.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/20850/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/#review41757
-----------------------------------------------------------
Looks great Alexandra, I made a few suggestions below, let me know what you think!
src/logging/logging.hpp
<https://reviews.apache.org/r/20850/#comment75338>
It doesn't look like the logging header needs to expose this?
What if we remove this function and inside 'initialize()' we directly use LOG_AT_LEVEL to indicate that logging is initialized? Keeping the comment would be great though to let others understand that we're forcing the creation of the log file.
src/master/master.cpp
<https://reviews.apache.org/r/20850/#comment75339>
What about having getLogFile take a logging::Flags object? Since it already returns a Try we could even return Error() instead of EXIT(1) when the level is invalid.
If we did this change, then logging::initialize could EXIT(1) when the level is invalid. It seems a bit unfortunate to have getLogSeverity call EXIT(1) when the input is invalid since it is exposed in the header and can be called from anywhere!
E.g.:
initialize(...)
{
...
if (flags.logging_level != "INFO" &&
flags.logging_level != "WARNING" &&
flags.logging_level != "ERROR") {
EXIT(1) << ...;
}
// We can hide getLogSeverity from the header and we can do an internal CHECK inside, instead of EXIT(1).
LogSeverity severity = getLogSeverity(flags.logging_level);
LOG_AT_LEVEL(severity) << "Logging level " + flags.logging_level + " initialized";
...
}
- Ben Mahler
On April 29, 2014, 4:06 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
> -----------------------------------------------------------
>
> (Updated April 29, 2014, 4:06 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>
> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>
> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>
> Update the documentation for 'logging_level' flag, with the values it can take.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/20850/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Ben Mahler <be...@gmail.com>.
> On May 7, 2014, 11:31 p.m., Ben Mahler wrote:
> > src/logging/logging.cpp, line 133
> > <https://reviews.apache.org/r/20850/diff/2/?file=572284#file572284line133>
> >
> > What are the implications of -1 here? A comment would be great, I'll update this to be INFO by default with a TODO.
>
> Alexandra Sava wrote:
> Well, since we support just three types of levels, that -1 means that the argument provided to that method is wrong.
What are the implications of -1, given how callers will pass the -1 to other methods (as was done in master.cpp and slave.cpp).
For example, what happens when logging::getLogFile is called with -1, will the program crash?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/#review42438
-----------------------------------------------------------
On May 1, 2014, 11:09 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 11:09 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>
> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>
> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>
> Update the documentation for 'logging_level' flag, with the values it can take.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/20850/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Alexandra Sava <al...@gmail.com>.
> On May 7, 2014, 11:31 p.m., Ben Mahler wrote:
> > Looks great Alexandra! I'll get this committed! There was a minor compile issue and some style issues that I'll get cleaned up for you.
Thanks for the adjustments Ben and reviews.
> On May 7, 2014, 11:31 p.m., Ben Mahler wrote:
> > src/logging/logging.cpp, line 133
> > <https://reviews.apache.org/r/20850/diff/2/?file=572284#file572284line133>
> >
> > What are the implications of -1 here? A comment would be great, I'll update this to be INFO by default with a TODO.
Well, since we support just three types of levels, that -1 means that the argument provided to that method is wrong.
> On May 7, 2014, 11:31 p.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 169-178
> > <https://reviews.apache.org/r/20850/diff/2/?file=572284#file572284line169>
> >
> > Why was this moved down? Shouldn't we be validating input prior to creating a directory?
> >
> > I'll move this up but please let me know if there was a reason to move it down!
Yes.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/#review42438
-----------------------------------------------------------
On May 1, 2014, 11:09 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 11:09 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>
> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>
> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>
> Update the documentation for 'logging_level' flag, with the values it can take.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/20850/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/#review42438
-----------------------------------------------------------
Ship it!
Looks great Alexandra! I'll get this committed! There was a minor compile issue and some style issues that I'll get cleaned up for you.
src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76242>
What are the implications of -1 here? A comment would be great, I'll update this to be INFO by default with a TODO.
src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76241>
Why was this moved down? Shouldn't we be validating input prior to creating a directory?
I'll move this up but please let me know if there was a reason to move it down!
src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76240>
Hm.. looks like this is missing a closing '"' character?
Let's align the newlines on the " character as well.
src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76239>
How about s/severityName/severity/ ? How about just doing it inline?
src/slave/slave.cpp
<https://reviews.apache.org/r/20850/#comment76238>
How about: s/logging_level/severity/ ?
Could we just call it inline?
Try<string> log = logging::getLogFile(
logging::getLogSeverity(flags.logging_level);
- Ben Mahler
On May 1, 2014, 11:09 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 11:09 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
>
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
>
> Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
>
> Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
>
> Update the documentation for 'logging_level' flag, with the values it can take.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/20850/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 20850: Customize the configuration of logging level
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/
-----------------------------------------------------------
(Updated May 1, 2014, 11:09 a.m.)
Review request for mesos.
Repository: mesos-git
Description
-------
Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
Update the documentation for 'logging_level' flag, with the values it can take.
Diffs (updated)
-----
src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
Diff: https://reviews.apache.org/r/20850/diff/
Testing
-------
Thanks,
Alexandra Sava
Re: Review Request 20850: Customize the configuration of logging level
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/
-----------------------------------------------------------
(Updated April 29, 2014, 4:06 p.m.)
Review request for mesos.
Repository: mesos-git
Description
-------
Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing flags.
Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
Disallow configuring FATAL logging because it's a special case and it involves too many changes across the entire source code.
Create validation for the 'logging_level' flag in order to notify the user if it had entered an unsupported value.
Update the documentation for 'logging_level' flag, with the values it can take.
Diffs
-----
src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
Diff: https://reviews.apache.org/r/20850/diff/
Testing
-------
Thanks,
Alexandra Sava