You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2015/12/29 23:18:14 UTC
Review Request 41782: Logger Module: Wire up the rotating container
logger module and test.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description
-------
Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
Adds the `RotatingContainerLogger` to the test module configuration.
Diffs
-----
src/Makefile.am bf71fe6ee65233bbc47a41f610d68ff9760e19f6
src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22
src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0
Diff: https://reviews.apache.org/r/41782/diff/
Testing
-------
Note: Some of the files added to the makefile are created in the next review.
Thanks,
Joseph Wu
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, line 111
> > <https://reviews.apache.org/r/41782/diff/2/?file=1181455#file1181455line111>
> >
> > s/sandboxPath/sandboxDirectory/
> >
> > ?
This one points to the sandbox module (the default one). I'll rename to `sandboxLoggerPath`.
> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, line 135
> > <https://reviews.apache.org/r/41782/diff/2/?file=1181455#file1181455line135>
> >
> > Interesting that you pulled it out as a constant in the previous review, but not here?
I was trying to stay consistent within the `module.cpp` file.
> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, lines 152-154
> > <https://reviews.apache.org/r/41782/diff/2/?file=1181455#file1181455line152>
> >
> > Is this a dead flag?
This flag tells the logger module to print its `stderr` to the given file. I added the while debugging, and kept it because it makes sense to have an option to *not* silence the logger's output.
(The logger can't write errors to the log it's writing to, because the errors it reports are due to writing to the log.)
> On Jan. 7, 2016, 4:43 p.m., Benjamin Hindman wrote:
> > src/tests/module.cpp, line 125
> > <https://reviews.apache.org/r/41782/diff/2/?file=1181455#file1181455line125>
> >
> > I'm confused by the name of the variable `truncatingPath` ... is this a remanant of the old code and should be `rotatingContainerLoggerPath`?
Oops. Yes that's a remnant.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/#review113373
-----------------------------------------------------------
On Jan. 8, 2016, 12:06 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41782/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2016, 12:06 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
>
>
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
> Adds the `RotatingContainerLogger` to the test module configuration.
>
>
> Diffs
> -----
>
> src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8
> src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c
> src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475
>
> Diff: https://reviews.apache.org/r/41782/diff/
>
>
> Testing
> -------
>
> Note: Some of the files added to the makefile are created in the next review.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/#review113373
-----------------------------------------------------------
src/Makefile.am (line 1077)
<https://reviews.apache.org/r/41782/#comment173972>
You should be able to fix the whitespace that review board is complaining about here, ditto below.
src/tests/module.cpp (line 111)
<https://reviews.apache.org/r/41782/#comment173951>
s/sandboxPath/sandboxDirectory/
?
src/tests/module.cpp (line 125)
<https://reviews.apache.org/r/41782/#comment173971>
I'm confused by the name of the variable `truncatingPath` ... is this a remanant of the old code and should be `rotatingContainerLoggerPath`?
src/tests/module.cpp (line 135)
<https://reviews.apache.org/r/41782/#comment173970>
Interesting that you pulled it out as a constant in the previous review, but not here?
src/tests/module.cpp (line 143)
<https://reviews.apache.org/r/41782/#comment173952>
.
src/tests/module.cpp (lines 152 - 154)
<https://reviews.apache.org/r/41782/#comment173954>
Is this a dead flag?
- Benjamin Hindman
On Jan. 5, 2016, 2:21 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41782/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 2:21 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
>
>
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
> Adds the `RotatingContainerLogger` to the test module configuration.
>
>
> Diffs
> -----
>
> src/Makefile.am e08e86724abaa3023f0483aa222354c95d4d3817
> src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22
> src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0
>
> Diff: https://reviews.apache.org/r/41782/diff/
>
>
> Testing
> -------
>
> Note: Some of the files added to the makefile are created in the next review.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/
-----------------------------------------------------------
(Updated Jan. 19, 2016, 7:41 p.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Changes
-------
Rename from "rotate" to "logrotate".
Summary (updated)
-----------------
Logger Module: Wire up the rotating container logger module and test.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description (updated)
-------
Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
Adds the RotatingContainerLogger to the test module configuration.
Diffs (updated)
-----
src/Makefile.am 4fabe600d4ba38c95a777d622b0b675dd5811a53
src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c
src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475
Diff: https://reviews.apache.org/r/41782/diff/
Testing
-------
Note: Some of the files added to the makefile are created in the next review.
Thanks,
Joseph Wu
Re: Review Request 41782: Wire up the rotating container logger
module and tests.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/
-----------------------------------------------------------
(Updated Jan. 15, 2016, 4:54 p.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Changes
-------
Change module flags to reflect the `logrotate` implementation.
Summary (updated)
-----------------
Wire up the rotating container logger module and tests.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description (updated)
-------
Wire up the rotating container logger module and tests.
Diffs (updated)
-----
src/Makefile.am d23e35001078a86775bd9b76baa207ecb9dab7e1
src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c
src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475
Diff: https://reviews.apache.org/r/41782/diff/
Testing
-------
Note: Some of the files added to the makefile are created in the next review.
Thanks,
Joseph Wu
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/#review114407
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On Jan. 14, 2016, 2:12 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41782/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 2:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
>
>
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
> Adds the `RotatingContainerLogger` to the test module configuration.
>
>
> Diffs
> -----
>
> src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660
> src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c
> src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475
>
> Diff: https://reviews.apache.org/r/41782/diff/
>
>
> Testing
> -------
>
> Note: Some of the files added to the makefile are created in the next review.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 6:12 p.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Changes
-------
Remove the `logger_log_filename` flag. Change type of `max_stdout_size` flag.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description
-------
Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
Adds the `RotatingContainerLogger` to the test module configuration.
Diffs (updated)
-----
src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660
src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c
src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475
Diff: https://reviews.apache.org/r/41782/diff/
Testing
-------
Note: Some of the files added to the makefile are created in the next review.
Thanks,
Joseph Wu
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/
-----------------------------------------------------------
(Updated Jan. 8, 2016, 12:06 p.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Changes
-------
Fixed some spacing, comments, and variable names.
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description
-------
Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
Adds the `RotatingContainerLogger` to the test module configuration.
Diffs (updated)
-----
src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8
src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c
src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475
Diff: https://reviews.apache.org/r/41782/diff/
Testing
-------
Note: Some of the files added to the makefile are created in the next review.
Thanks,
Joseph Wu
Re: Review Request 41782: Logger Module: Wire up the rotating
container logger module and test.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41782/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 6:21 p.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Changes
-------
Update according to changes made in the next review (different parameters, more files).
Bugs: MESOS-4136
https://issues.apache.org/jira/browse/MESOS-4136
Repository: mesos
Description
-------
Creates a new binary "mesos-rotate-logger" for use by the non-default "Rotating" ContainerLogger module.
Adds the `RotatingContainerLogger` to the test module configuration.
Diffs (updated)
-----
src/Makefile.am e08e86724abaa3023f0483aa222354c95d4d3817
src/tests/module.hpp 8e92774ddd51bc8a1368fb1cf6546300696b2d22
src/tests/module.cpp 7968519996ca9f9d8895e73d5f173d26a7e794e0
Diff: https://reviews.apache.org/r/41782/diff/
Testing
-------
Note: Some of the files added to the makefile are created in the next review.
Thanks,
Joseph Wu