You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by David Robinson <dr...@twitter.com> on 2014/12/16 01:51:31 UTC
Review Request 29071: added webui_log option
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/
-----------------------------------------------------------
Review request for mesos.
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
Re: Review Request 29071: added webui_log option
Posted by David Robinson <dr...@twitter.com>.
> On Dec. 16, 2014, 12:58 a.m., Dominic Hamon wrote:
> > src/slave/slave.cpp, line 459
> > <https://reviews.apache.org/r/29071/diff/1/?file=792487#file792487line459>
> >
> > might want to check that the file exists, as per the log_dir stanza above.
The stanza above just returns a filename (created from log_dir, log level etc), it doesn't check that the file exists. FilesProcess::attach is where file existance check occurs:
Future<Nothing> FilesProcess::attach(const string& path, const string& name)
{
Result<string> result = os::realpath(path);
if (!result.isSome()) {
return Failure(
"Failed to get realpath of '" + path + "': " +
(result.isError()
? result.error()
: "No such file or directory"));
}
// Make sure we have permissions to read the file/dir.
...
> On Dec. 16, 2014, 12:58 a.m., Dominic Hamon wrote:
> > src/slave/slave.cpp, line 446
> > <https://reviews.apache.org/r/29071/diff/1/?file=792487#file792487line446>
> >
> > so webui_log overrides log_dir? should check if both are set and error.
Done.
> On Dec. 16, 2014, 12:58 a.m., Dominic Hamon wrote:
> > src/webui/master/static/js/controllers.js, line 405
> > <https://reviews.apache.org/r/29071/diff/1/?file=792488#file792488line405>
> >
> > or 'webui_log'
Done.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review65162
-----------------------------------------------------------
On Dec. 16, 2014, 12:51 a.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 12:51 a.m.)
>
>
> Review request for mesos.
>
>
> 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
>
>
Re: Review Request 29071: added webui_log option
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review65162
-----------------------------------------------------------
src/slave/slave.cpp
<https://reviews.apache.org/r/29071/#comment108150>
so webui_log overrides log_dir? should check if both are set and error.
src/slave/slave.cpp
<https://reviews.apache.org/r/29071/#comment108149>
might want to check that the file exists, as per the log_dir stanza above.
src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/29071/#comment108151>
or 'webui_log'
- Dominic Hamon
On Dec. 15, 2014, 4:51 p.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2014, 4:51 p.m.)
>
>
> Review request for mesos.
>
>
> 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
>
>
Re: Review Request 29071: added webui_log option
Posted by David Robinson <dr...@twitter.com>.
> On Dec. 16, 2014, 1:44 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> >
> > Reviews applied: [29071]
> >
> > Failed command: ./support/apply-review.sh -n -r 29071
> >
> > Error:
> > 2014-12-16 01:44:48 URL:https://reviews.apache.org/r/29071/diff/raw/ [5226/5226] -> "29071.patch" [1]
> > error: master/flags.hpp: does not exist in index
> > error: master/http.cpp: does not exist in index
> > error: master/master.cpp: does not exist in index
> > error: slave/flags.hpp: does not exist in index
> > error: slave/http.cpp: does not exist in index
> > error: slave/slave.cpp: does not exist in index
> > error: webui/master/static/js/controllers.js: does not exist in index
> > Failed to apply patch
Guess you don't like 'noprefix = true'?!
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review65165
-----------------------------------------------------------
On Dec. 16, 2014, 10:37 p.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 10:37 p.m.)
>
>
> Review request for mesos.
>
>
> 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
>
>
Re: Review Request 29071: added webui_log option
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review65165
-----------------------------------------------------------
Bad patch!
Reviews applied: [29071]
Failed command: ./support/apply-review.sh -n -r 29071
Error:
2014-12-16 01:44:48 URL:https://reviews.apache.org/r/29071/diff/raw/ [5226/5226] -> "29071.patch" [1]
error: master/flags.hpp: does not exist in index
error: master/http.cpp: does not exist in index
error: master/master.cpp: does not exist in index
error: slave/flags.hpp: does not exist in index
error: slave/http.cpp: does not exist in index
error: slave/slave.cpp: does not exist in index
error: webui/master/static/js/controllers.js: does not exist in index
Failed to apply patch
- Mesos ReviewBot
On Dec. 16, 2014, 12:51 a.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 12:51 a.m.)
>
>
> Review request for mesos.
>
>
> 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
>
>
Re: Review Request 29071: added webui_log option
Posted by David Robinson <dr...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review69433
-----------------------------------------------------------
Ping?
- David Robinson
On Dec. 16, 2014, 10:37 p.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 10:37 p.m.)
>
>
> Review request for mesos.
>
>
> 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
>
>
Re: Review Request 29071: added webui_log option
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review69767
-----------------------------------------------------------
Bad patch!
Reviews applied: [29071]
Failed command: ./support/apply-review.sh -n -r 29071
Error:
2015-01-27 08:43:35 URL:https://reviews.apache.org/r/29071/diff/raw/ [8592/8592] -> "29071.patch" [1]
error: patch failed: src/master/flags.hpp:152
error: src/master/flags.hpp: patch does not apply
error: patch failed: src/master/http.cpp:530
error: src/master/http.cpp: patch does not apply
error: patch failed: src/master/master.cpp:626
error: src/master/master.cpp: patch does not apply
error: patch failed: src/slave/flags.hpp:460
error: src/slave/flags.hpp: patch does not apply
error: patch failed: src/slave/http.cpp:371
error: src/slave/http.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:446
error: src/slave/slave.cpp: patch does not apply
error: patch failed: src/webui/master/static/js/controllers.js:334
error: src/webui/master/static/js/controllers.js: patch does not apply
Failed to apply patch
- Mesos ReviewBot
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/logging/flags.hpp 11efb84cc2c509f852f8ba20f16a366b4cb5810f
> src/master/flags.hpp 6c18a1af625311ef149b5877b08f63c2b12c040d
> src/master/http.cpp 46890bed05d7c4b63e1f7be5bb35217173e0ade8
> src/master/master.cpp ab6d1d17367f199191b7c77bccec73ec3b112d4f
> src/slave/flags.hpp 0f6cc41d60a2e3bc2121cc438351135541ef99ba
> src/slave/http.cpp d1cf8a68fab9a2df44f6c753683ad37fd4b1a1f9
> src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/29071/diff/
>
>
> Testing
> -------
>
> Ran locally.
>
>
> Thanks,
>
> David Robinson
>
>
Re: Review Request 29071: added webui_log option
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 29071: added webui_log option
Posted by David Robinson <dr...@twitter.com>.
-----------------------------------------------------------
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.
Changes
-------
assigning it to @bmahler as he thought a bunch about this. -- @vinodkone
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
Re: Review Request 29071: added webui_log option
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/#review65259
-----------------------------------------------------------
Patch looks great!
Reviews applied: [29071]
All tests passed.
- Mesos ReviewBot
On Dec. 16, 2014, 10:37 p.m., David Robinson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29071/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2014, 10:37 p.m.)
>
>
> Review request for mesos.
>
>
> 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
>
>
Re: Review Request 29071: added webui_log option
Posted by David Robinson <dr...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29071/
-----------------------------------------------------------
(Updated Dec. 16, 2014, 10:37 p.m.)
Review request for mesos.
Changes
-------
Dominic's feedback.
Bugs: MESOS-2193
https://issues.apache.org/jira/browse/MESOS-2193
Repository: mesos-git
Description
-------
added webui_log option
Diffs (updated)
-----
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