-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review69709
-----------------------------------------------------------
Thanks Drob, just some minor things and we'll be good to go.
Could we add a NOTE to the help message for --log_dir to mention the stderr
issue?
```
add(&Flags::log_dir,
"log_dir",
"Directory path to put log files (no default, nothing\n"
"is written to disk unless specified;\n"
"does not affect logging to stderr).\n"
"NOTE: 3rd party log messages (e.g. ZooKeeper) are\n"
"only written to stderr!\n");
```
src/master/flags.hpp
<https://reviews.apache.org/r/29071/#comment114440>
How about `--external_log_file`? I'm thinking we should name this what it
is, rather than what it is used for, because it's not only for the webui, it's
also for the HTTP api (GET /files/...):
```
TODO(drobinson): Consider moving this up to logging/flags.hpp.
add(&Flags::external_log_file,
"external_log_file",
"Specified the externally managed log file. This file will be\n"
"exposed in the webui and HTTP api. This is useful when using\n"
"stderr logging as the log file is otherwise unknown to Mesos.");
```
Ditto for the slave side. Thoughts?
src/master/master.cpp
<https://reviews.apache.org/r/29071/#comment114434>
Hm.. it seems like `--webui_log` should take priority over `--log_dir`, no?
Could we just do:
```
// Expose the log file for the webui. Fall back to 'log_dir' if
// an explicit file was not specified.
if (flags.webui_log.isSome()) {
...
} else if (flags.log_dir.isSome()) {
...
}
```
src/master/master.cpp
<https://reviews.apache.org/r/29071/#comment114436>
Mind wrapping it like this to match the other .onAny's?
```
files->attach(flags.webui_log.get(), "/master/log")
.onAny(defer(self(),
&Self::fileAttached,
lambda::_1,
flags.webui_log.get()));
```
src/slave/slave.cpp
<https://reviews.apache.org/r/29071/#comment114438>
Ditto here.
src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/29071/#comment114442>
If `external_log_file` ends up overriding `log_dir` instead of being
mutually exclusive with it, I would re-order this to look at it first.
- Ben Mahler
On Jan. 23, 2015, 8:14 p.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2015, 8:14 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2193
> https://issues.apache.org/jira/browse/MESOS-2193
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> added webui_log option
>
>
> Diffs
> -----
>
> src/master/flags.hpp 1cea50c02f3ad7de1e1ae91d65d1accdb9af7b03
> src/master/http.cpp 46890bed05d7c4b63e1f7be5bb35217173e0ade8
> src/master/master.cpp 0f55a5cc2d6845cbaace718a48f771d80aad0e6e
> src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef
> src/slave/http.cpp d1cf8a68fab9a2df44f6c753683ad37fd4b1a1f9
> src/slave/slave.cpp 50b57819b55bdcdb9f49f20648199badc4d3f37b
> src/webui/master/static/js/controllers.js
> 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/29071/diff/
>
>
> Testing
> -------
>
> Ran locally.
>
>
> Thanks,
>
> David Robinson
>
>
