You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Marco Massenzio <ma...@mesosphere.io> on 2015/10/19 20:27:02 UTC
Review Request 39447: MESOS-3692 - better documented --switch_user
flag
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Bugs: MESOS-3692
https://issues.apache.org/jira/browse/MESOS-3692
Repository: mesos
Description
-------
MESOS-3692 - better documented --switch_user flag
Diffs
-----
docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
Diff: https://reviews.apache.org/r/39447/diff/
Testing
-------
Thanks,
Marco Massenzio
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103142
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39447]
All tests passed.
- Mesos ReviewBot
On Oct. 19, 2015, 7:06 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 7:06 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 11:55 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Updated the message to reflect that the task fails.
Bugs: MESOS-3692
https://issues.apache.org/jira/browse/MESOS-3692
Repository: mesos
Description
-------
MESOS-3692 - better documented --switch_user flag
Added better logging around the point where the error may occur.
Diffs (updated)
-----
docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
Diff: https://reviews.apache.org/r/39447/diff/
Testing
-------
make check
Thanks,
Marco Massenzio
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 10:21 p.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-3692
https://issues.apache.org/jira/browse/MESOS-3692
Repository: mesos
Description
-------
MESOS-3692 - better documented --switch_user flag
Added better logging around the point where the error may occur.
Diffs (updated)
-----
docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
Diff: https://reviews.apache.org/r/39447/diff/
Testing
-------
make check
Thanks,
Marco Massenzio
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
> On Oct. 19, 2015, 10:09 p.m., Michael Park wrote:
> > docs/configuration.md, line 1445
> > <https://reviews.apache.org/r/39447/diff/4/?file=1101271#file1101271line1445>
> >
> > `\Could not chown work directory\` -- is this intentional? If yes, what does it do...?
good catch!
copy & paste & find & replace fail :)
- Marco
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103170
-----------------------------------------------------------
On Oct. 19, 2015, 9:27 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 9:27 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103170
-----------------------------------------------------------
docs/configuration.md (line 1442)
<https://reviews.apache.org/r/39447/#comment161136>
How about `s/submitted/launched/`?
docs/configuration.md (line 1445)
<https://reviews.apache.org/r/39447/#comment161133>
`\Could not chown work directory\` -- is this intentional? If yes, what does it do...?
docs/configuration.md (line 1448)
<https://reviews.apache.org/r/39447/#comment161134>
`s/Agent/agent/`
src/slave/paths.cpp (line 427)
<https://reviews.apache.org/r/39447/#comment161138>
`s/non existing/nonexistent/`
`s/Agent/agent/`
src/slave/paths.cpp (line 428)
<https://reviews.apache.org/r/39447/#comment161139>
Wrap `--switch_user` in ```
- Michael Park
On Oct. 19, 2015, 9:27 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 9:27 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103163
-----------------------------------------------------------
Ship it!
Ship It!
- Guangya Liu
On 十月 19, 2015, 9:27 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated 十月 19, 2015, 9:27 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 9:27 p.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-3692
https://issues.apache.org/jira/browse/MESOS-3692
Repository: mesos
Description
-------
MESOS-3692 - better documented --switch_user flag
Added better logging around the point where the error may occur.
Diffs (updated)
-----
docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
Diff: https://reviews.apache.org/r/39447/diff/
Testing
-------
make check
Thanks,
Marco Massenzio
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 9:23 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
comments addressed
Bugs: MESOS-3692
https://issues.apache.org/jira/browse/MESOS-3692
Repository: mesos
Description
-------
MESOS-3692 - better documented --switch_user flag
Added better logging around the point where the error may occur.
Diffs (updated)
-----
3rdparty/libprocess/README.md 8e434c68a2c9bead608324b0e9ad0d31a2a3bb08
docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
docs/upgrades.md 3854e59f68d53892b2673a4542ea2aaf96936cbb
src/examples/persistent_volume_framework.cpp 176ac3df6f6a536affb4853d5d2f0bfda81fd720
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
Diff: https://reviews.apache.org/r/39447/diff/
Testing
-------
make check
Thanks,
Marco Massenzio
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
> On Oct. 19, 2015, 7:35 p.m., Michael Park wrote:
> > Thanks for doing this! It's definitely clearer (at least to me) as to what this flag is controlling.
> >
> > A few general comments here:
> > (1) We should be using ``` in some of the cases instead of `'` right? It seems to be a bit inconsistent currently.
> > (2) Based on existing documentation, I don't think we should go out of our way to capitalize `Agent`.
agreed - tried to use `` consistently, and replaced '
- Marco
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103146
-----------------------------------------------------------
On Oct. 19, 2015, 7:06 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 7:06 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103146
-----------------------------------------------------------
Thanks for doing this! It's definitely clearer (at least to me) as to what this flag is controlling.
A few general comments here:
(1) We should be using ``` in some of the cases instead of `'` right? It seems to be a bit inconsistent currently.
(2) Based on existing documentation, I don't think we should go out of our way to capitalize `Agent`.
docs/configuration.md (line 1441)
<https://reviews.apache.org/r/39447/#comment161091>
`s/(its default value)//`
docs/configuration.md (line 1443)
<https://reviews.apache.org/r/39447/#comment161098>
> rather than the one running the Agent
Let's just mention this once in the `false` case.
docs/configuration.md (line 1444)
<https://reviews.apache.org/r/39447/#comment161092>
`s/, obviously,//`
docs/configuration.md (line 1445)
<https://reviews.apache.org/r/39447/#comment161094>
`s/exist/exist,/`
docs/configuration.md (line 1446)
<https://reviews.apache.org/r/39447/#comment161099>
How about we keep it symmetric? i.e. `If set to `true`, ... . If set to `false`, ... .`?
- Michael Park
On Oct. 19, 2015, 7:06 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 7:06 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
> On Oct. 19, 2015, 7:27 p.m., Kapil Arya wrote:
> > src/slave/paths.cpp, lines 427-429
> > <https://reviews.apache.org/r/39447/diff/2/?file=1101169#file1101169line427>
> >
> > Why not use the same `LOG(WARNING)` statement? That way the entire message will appear together instead of being sliced up by some other concurrent message.
Because a priori I don't know what `chown.error()` will emit (can be a multiline error, whatever) and so it may look pretty awful.
However, having looked at it - I've actually moved it to the end of the sentence, so it should still look good.
- Marco
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103148
-----------------------------------------------------------
On Oct. 19, 2015, 7:06 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 7:06 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/#review103148
-----------------------------------------------------------
Ship it!
Gave it a go :)
docs/configuration.md (line 1441)
<https://reviews.apache.org/r/39447/#comment161095>
The "(its default value)" is redundant wrt "(default: true)" at the end of the help message. Can we kill this?
s/ the Agent/, the Agent/
docs/configuration.md (line 1444)
<https://reviews.apache.org/r/39447/#comment161096>
Kill `obviously`?
docs/configuration.md (line 1445)
<https://reviews.apache.org/r/39447/#comment161097>
s/exist a/exist, a/
src/slave/flags.cpp (lines 166 - 169)
<https://reviews.apache.org/r/39447/#comment161101>
Same comments as for the doc.
src/slave/paths.cpp (lines 427 - 429)
<https://reviews.apache.org/r/39447/#comment161100>
Why not use the same `LOG(WARNING)` statement? That way the entire message will appear together instead of being sliced up by some other concurrent message.
- Kapil Arya
On Oct. 19, 2015, 3:06 p.m., Marco Massenzio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39447/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2015, 3:06 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3692
> https://issues.apache.org/jira/browse/MESOS-3692
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MESOS-3692 - better documented --switch_user flag
> Added better logging around the point where the error may occur.
>
>
> Diffs
> -----
>
> docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
> src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
> src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
>
> Diff: https://reviews.apache.org/r/39447/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Marco Massenzio
>
>
Re: Review Request 39447: MESOS-3692 - better documented
--switch_user flag
Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39447/
-----------------------------------------------------------
(Updated Oct. 19, 2015, 7:06 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Added better logging around the point where the error may occur
Bugs: MESOS-3692
https://issues.apache.org/jira/browse/MESOS-3692
Repository: mesos
Description (updated)
-------
MESOS-3692 - better documented --switch_user flag
Added better logging around the point where the error may occur.
Diffs (updated)
-----
docs/configuration.md 69fb37fc38e52277605b82cf67067168a37d79a6
src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2
src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374
Diff: https://reviews.apache.org/r/39447/diff/
Testing (updated)
-------
make check
Thanks,
Marco Massenzio