Re: Review Request 72945: Ignored the directoy `/dev/nvidia-caps` when globing Nvidia GPU devices.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72945/#review222174 --- Here is a reference as to what this folder is used for: https://docs.google.com/document/d/194A-Hg3mLlIW4eo2BSUcGKpzZf2ciX47eoH5T-WZNXo/edit# - Kevin Klues On Oct. 13, 2020, 2:31 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72945/ > --- > > (Updated Oct. 13, 2020, 2:31 a.m.) > > > Review request for mesos, Benjamin Mahler and Kevin Klues. > > > Bugs: MESOS-10192 > https://issues.apache.org/jira/browse/MESOS-10192 > > > Repository: mesos > > > Description > --- > > The directory `/dev/nvidia-caps` was introduced in CUDA 11.0, just > ignore it since we only care about the Nvidia GPU device files. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/gpu/isolator.cpp > a0be1026bdaf7d4bb33f41e4c7d45666bd61f005 > > > Diff: https://reviews.apache.org/r/72945/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Qian Zhang > >
Re: Review Request 70016: Supported nvidia-docker 2.0 for CUDA 10+.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70016/#review212986 --- src/slave/containerizer/mesos/isolators/gpu/isolator.cpp Lines 418 (patched) <https://reviews.apache.org/r/70016/#comment298847> This has the same limitations that the original nvidia-docker does, it's just that we inject these envvars here now instead of relying on the docker image to do it. If you really want to change to the true nvidia-docker2 model, you need to follow the logic in https://github.com/NVIDIA/libnvidia-container The libraries are no longer injected in /usr/lib/nvidia, but rather in default lib locations, with ldcache inside the container being updated after the injection. src/slave/containerizer/mesos/isolators/gpu/volume.cpp Lines 203-249 (patched) <https://reviews.apache.org/r/70016/#comment298846> This feels like a big hack. I guess the main idea though, is to do what the original nvidia-docker used to do in the container image with PATH and LD_LIBRARY_PATH (including all of its limitations), but do it manually in Mesos if / when we detect that a GPU is needed by a container. - Kevin Klues On Feb. 20, 2019, 6:26 a.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70016/ > --- > > (Updated Feb. 20, 2019, 6:26 a.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, and Kevin Klues. > > > Bugs: MESOS-9549 > https://issues.apache.org/jira/browse/MESOS-9549 > > > Repository: mesos > > > Description > --- > > nvidia-docker 2.0, which is used by CUDA 10+, moves some of the runtime > injection that was originally done in the image to its new nvidia > container runtime. To adapt this change, we adjusted the binaries and > libraries and injected the `PATH` and `LD_LIBRARY_PATH` environment > variables in the `gpu/nvidia` isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/gpu/isolator.cpp > f39e7c3d1ccfe097116fe59b05c22fbb3f83b698 > src/slave/containerizer/mesos/isolators/gpu/volume.hpp > e71fe95234ff10c72cfaa4ad39591f70a531c383 > src/slave/containerizer/mesos/isolators/gpu/volume.cpp > 0d0d778d6a8467c1ac87286e75d47faf8243afa4 > > > Diff: https://reviews.apache.org/r/70016/diff/1/ > > > Testing > --- > > `sudo make check` > > More testing done later in chain. > > > Thanks, > > Chun-Hung Hsiao > >
Re: Review Request 69395: Added '--all' flag to 'mesos task list'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69395/#review210864 --- src/python/cli_new/lib/cli/tests/task.py Lines 250 (patched) <https://reviews.apache.org/r/69395/#comment295665> Can we be more descriptive here? src/python/cli_new/lib/cli/tests/task.py Lines 252 (patched) <https://reviews.apache.org/r/69395/#comment295666> Comment should mention launching two tasks or something. - Kevin Klues On Nov. 26, 2018, 1:51 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69395/ > --- > > (Updated Nov. 26, 2018, 1:51 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > With this option, the command can show all the tasks that have ever been > run. This makes the command's behavior closer to the one of 'docker ps'. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0ce21b365d64c5c331ce7f1a5937258960e15b6f > src/python/cli_new/lib/cli/tests/task.py > eb01bf5cae095a067572f534a13f4ec542101ca5 > > > Diff: https://reviews.apache.org/r/69395/diff/4/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > test_list_all (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 8 tests in 20.889s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69395: Added '--all' flag to 'mesos task list'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69395/#review210866 --- Ship it! Ship It! - Kevin Klues On Nov. 26, 2018, 1:54 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69395/ > --- > > (Updated Nov. 26, 2018, 1:54 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > With this option, the command can show all the tasks that have ever been > run. This makes the command's behavior closer to the one of 'docker ps'. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0ce21b365d64c5c331ce7f1a5937258960e15b6f > src/python/cli_new/lib/cli/tests/task.py > eb01bf5cae095a067572f534a13f4ec542101ca5 > > > Diff: https://reviews.apache.org/r/69395/diff/5/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > test_list_all (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 8 tests in 20.889s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69426/#review210863
---
src/python/cli_new/lib/cli/tests/base.py
Lines 516 (patched)
<https://reviews.apache.org/r/69426/#comment295658>
Let's return the task here. I'm not 100% sure when / how we will use it,
but it's better than just retunring `None` in my opinion.
src/python/cli_new/lib/cli/tests/task.py
Lines 70 (patched)
<https://reviews.apache.org/r/69426/#comment295659>
Add the name of the task in the error message.
src/python/cli_new/lib/cli/tests/task.py
Lines 77 (patched)
<https://reviews.apache.org/r/69426/#comment295661>
This could fail for two reasons (which is fine), but we should change the
error message to reflect exactly what's gone wrong (i.e. that we can't get the
tasks from that endpoint -- not just that we couldn't open it).
src/python/cli_new/lib/cli/tests/task.py
Lines 78 (patched)
<https://reviews.apache.org/r/69426/#comment295660>
Let's put a line break here.
src/python/cli_new/lib/cli/tests/task.py
Lines 96-99 (original), 108-122 (patched)
<https://reviews.apache.org/r/69426/#comment295662>
Same comments as above.
src/python/cli_new/lib/cli/tests/task.py
Lines 129-132 (original), 152-166 (patched)
<https://reviews.apache.org/r/69426/#comment295663>
Same comments as above.
src/python/cli_new/lib/cli/tests/task.py
Lines 164-168 (original), 198-210 (patched)
<https://reviews.apache.org/r/69426/#comment295664>
Same comments as above.
- Kevin Klues
On Nov. 25, 2018, 11:08 vorm., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69426/
> ---
>
> (Updated Nov. 25, 2018, 11:08 vorm.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Replaces 'running_tasks(master)', a function that was not generic nor
> explicit, by 'wait_for_task(master, name, state, delay)'. This helper
> function waits a 'delay' for a task with a given 'name' to be in a
> certain 'state'.
>
> All uses of 'running_tasks' have been replaced by the new function.
>
>
> Diffs
> -
>
> src/python/cli_new/lib/cli/tests/base.py
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40
> src/python/cli_new/lib/cli/tests/task.py
> 1b48c0a4729043c874330552074b05368e38b715
>
>
> Diff: https://reviews.apache.org/r/69426/diff/2/
>
>
> Testing
> ---
>
> ```
> (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
>
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
>
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
>
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
>
> --
> Ran 7 tests in 19.319s
>
> OK
> ```
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 69425: Fixed name of task created when running mesos-cli-tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69425/#review210862 --- Ship it! Ship It! - Kevin Klues On Nov. 22, 2018, 11:33 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69425/ > --- > > (Updated Nov. 22, 2018, 11:33 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Fixed name of task created when running mesos-cli-tests. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > c28e2a65b5c1b881c2f4eb09baf9806338f26e40 > > > Diff: https://reviews.apache.org/r/69425/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.732s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69394: Updated 'mesos task list' to only display running tasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69394/#review210860 --- Ship it! Ship It! - Kevin Klues On Nov. 25, 2018, 9:53 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69394/ > --- > > (Updated Nov. 25, 2018, 9:53 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Updated 'mesos task list' to only display running tasks. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 4abc70a2de7e63bb629d5ed82d0045b29fb237fa > src/python/cli_new/lib/cli/tests/task.py > 1b48c0a4729043c874330552074b05368e38b715 > > > Diff: https://reviews.apache.org/r/69394/diff/2/ > > > Testing > --- > > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.845s > > OK > > > Thanks, > > Armand Grillet > >
Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69426/#review210813 --- src/python/cli_new/lib/cli/tests/task.py Lines 66-68 (original), 68-70 (patched) <https://reviews.apache.org/r/69426/#comment295576> Instead of throwing an exception here, do an assert that the tasks array has 1 element in it. Do the same for all instances below. - Kevin Klues On Nov. 22, 2018, 11:35 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69426/ > --- > > (Updated Nov. 22, 2018, 11:35 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Replaces 'running_tasks(master)', a function that was not generic nor > explicit, by 'wait_for_task(master, name, state, delay)'. This helper > function waits a 'delay' for a task with a given 'name' to be in a > certain 'state'. > > All uses of 'running_tasks' have been replaced by the new function. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > c28e2a65b5c1b881c2f4eb09baf9806338f26e40 > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69426/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.319s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69426/#review210811 --- src/python/cli_new/lib/cli/tests/base.py Line 506 (original), 507 (patched) <https://reviews.apache.org/r/69426/#comment295570> Add a default dealy of 1 here, so that tests don't always have to write it in. src/python/cli_new/lib/cli/tests/base.py Line 521 (original), 522 (patched) <https://reviews.apache.org/r/69426/#comment295571> This should be a CLIException and should have a string saying: ``` "Timed out waiting for task" ``` src/python/cli_new/lib/cli/tests/task.py Lines 65-68 (original), 66-70 (patched) <https://reviews.apache.org/r/69426/#comment295572> The new `wait_or_task` call should be wrapped in a `try` and the `CLIException` it raises should be composed according to the normal composition rules we use for exceptions. We also don't need the delay set explicitly to `1` anymore if you make the change mentioned in the previous file. Apply this to all instance of this pattern below as well. - Kevin Klues On Nov. 22, 2018, 11:35 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69426/ > --- > > (Updated Nov. 22, 2018, 11:35 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Replaces 'running_tasks(master)', a function that was not generic nor > explicit, by 'wait_for_task(master, name, state, delay)'. This helper > function waits a 'delay' for a task with a given 'name' to be in a > certain 'state'. > > All uses of 'running_tasks' have been replaced by the new function. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > c28e2a65b5c1b881c2f4eb09baf9806338f26e40 > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69426/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.319s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69425: Fixed name of task created when running mesos-cli-tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69425/#review210810 --- Ship it! Ship It! - Kevin Klues On Nov. 22, 2018, 11:33 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69425/ > --- > > (Updated Nov. 22, 2018, 11:33 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Fixed name of task created when running mesos-cli-tests. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > c28e2a65b5c1b881c2f4eb09baf9806338f26e40 > > > Diff: https://reviews.apache.org/r/69425/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.732s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69390: Added docs describing how to use the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69390/#review210804 --- Ship it! I have fixed all the issues listed above myself before committing. - Kevin Klues On Nov. 20, 2018, 11:12 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69390/ > --- > > (Updated Nov. 20, 2018, 11:12 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9333 > https://issues.apache.org/jira/browse/MESOS-9333 > > > Repository: mesos > > > Description > --- > > The documentation describes the main commands of the new CLI, how to > activate it, how to build Mesos including this component, and how to > write a configuration file for it. > > > Diffs > - > > docs/cli.md PRE-CREATION > docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 > > > Diff: https://reviews.apache.org/r/69390/diff/3/ > > > Testing > --- > > Checked the rendered Markdown code. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69394: Updated 'mesos task list' to only display running tasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69394/#review210808 --- Ship it! I will make the change marked above before committing. - Kevin Klues On Nov. 19, 2018, 2:17 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69394/ > --- > > (Updated Nov. 19, 2018, 2:17 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Updated 'mesos task list' to only display running tasks. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0536ccb29551400ce59713017c86eb5e858aeb5f > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69394/diff/1/ > > > Testing > --- > > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.845s > > OK > > > Thanks, > > Armand Grillet > >
Re: Review Request 69394: Updated 'mesos task list' to only display running tasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69394/#review210807 --- src/python/cli_new/lib/cli/tests/task.py Line 189 (original), 189 (patched) <https://reviews.apache.org/r/69394/#comment295567> We should use `[-1]` here instead of `pop()`. - Kevin Klues On Nov. 19, 2018, 2:17 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69394/ > --- > > (Updated Nov. 19, 2018, 2:17 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Updated 'mesos task list' to only display running tasks. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0536ccb29551400ce59713017c86eb5e858aeb5f > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69394/diff/1/ > > > Testing > --- > > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 19.845s > > OK > > > Thanks, > > Armand Grillet > >
Re: Review Request 69393: Displayed 'State' field when using 'mesos task list'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69393/#review210805 --- src/python/cli_new/lib/cli/tests/task.py Line 188 (original), 189 (patched) <https://reviews.apache.org/r/69393/#comment295566> This is racy, as the task may have reached RUNNING before reaching this line. You have told me offline that this is fixed in a follow-up RR, so we can address this then. - Kevin Klues On Nov. 19, 2018, 4:14 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69393/ > --- > > (Updated Nov. 19, 2018, 4:14 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Displayed 'State' field when using 'mesos task list'. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0536ccb29551400ce59713017c86eb5e858aeb5f > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69393/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 20.812s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69393: Displayed 'State' field when using 'mesos task list'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69393/#review210806 --- Ship it! Ship It! - Kevin Klues On Nov. 19, 2018, 4:14 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69393/ > --- > > (Updated Nov. 19, 2018, 4:14 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Displayed 'State' field when using 'mesos task list'. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0536ccb29551400ce59713017c86eb5e858aeb5f > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69393/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 20.812s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69390: Added docs describing how to use the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69390/#review210801 --- docs/cli.md Lines 7 (patched) <https://reviews.apache.org/r/69390/#comment295560> Should only have 1 line here. docs/cli.md Lines 23 (patched) <https://reviews.apache.org/r/69390/#comment295561> However, ... docs/cli.md Lines 46 (patched) <https://reviews.apache.org/r/69390/#comment295562> s/where are the masters of the cluster/where the masters of the cluste are/ docs/cli.md Lines 47 (patched) <https://reviews.apache.org/r/69390/#comment295564> s/and/as well as/ s/the plugins/any plugins/ s/on top of/in addition to/ docs/cli.md Lines 52 (patched) <https://reviews.apache.org/r/69390/#comment295563> The plugin array lists... docs/cli.md Lines 59 (patched) <https://reviews.apache.org/r/69390/#comment295565> The master field is composed of... - Kevin Klues On Nov. 20, 2018, 11:12 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69390/ > --- > > (Updated Nov. 20, 2018, 11:12 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9333 > https://issues.apache.org/jira/browse/MESOS-9333 > > > Repository: mesos > > > Description > --- > > The documentation describes the main commands of the new CLI, how to > activate it, how to build Mesos including this component, and how to > write a configuration file for it. > > > Diffs > - > > docs/cli.md PRE-CREATION > docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 > > > Diff: https://reviews.apache.org/r/69390/diff/3/ > > > Testing > --- > > Checked the rendered Markdown code. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69390: Added docs describing how to use the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69390/#review210703 --- docs/cli.md Lines 20 (patched) <https://reviews.apache.org/r/69390/#comment295438> We should move the text that says: ``` For now, the Mesos Command Line Interface is still under development and not built as part of a standard Mesos distribution. ``` to here and have a better opening line for the whole document. - Kevin Klues On Nov. 20, 2018, 10:57 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69390/ > --- > > (Updated Nov. 20, 2018, 10:57 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9333 > https://issues.apache.org/jira/browse/MESOS-9333 > > > Repository: mesos > > > Description > --- > > The documentation describes the main commands of the CLI, how to enable > it and how to build Mesos including this component. > > > Diffs > - > > docs/cli.md PRE-CREATION > docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 > > > Diff: https://reviews.apache.org/r/69390/diff/2/ > > > Testing > --- > > Checked the rendered Markdown code. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69255: Updated PyInstaller requirement for new CLI to support Python 3.7.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69255/#review210664 --- Ship it! Ship It! - Kevin Klues On Nov. 5, 2018, 6:13 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69255/ > --- > > (Updated Nov. 5, 2018, 6:13 nachm.) > > > Review request for mesos, Benjamin Bannier and Kevin Klues. > > > Bugs: MESOS-9370 > https://issues.apache.org/jira/browse/MESOS-9370 > > > Repository: mesos > > > Description > --- > > Updated PyInstaller requirement for new CLI to support Python 3.7. > > > Diffs > - > > src/python/cli_new/pip-requirements.txt > 520dc6131cf05e8f4626a1c673be9b6a59766a8d > > > Diff: https://reviews.apache.org/r/69255/diff/2/ > > > Testing > --- > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ PYTHON_3=python37 ../configure --enable-new-cli > $ make check > $ ./src/mesos > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69395: Added '--all' flag to 'mesos task list'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69395/#review210662 --- src/python/cli_new/lib/cli/tests/task.py Lines 238 (patched) <https://reviews.apache.org/r/69395/#comment295387> This will be flakey. - Kevin Klues On Nov. 19, 2018, 2:17 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69395/ > --- > > (Updated Nov. 19, 2018, 2:17 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > With this option, the command can show all the tasks that have ever been > run. This makes the command's behavior closer to the one of 'docker ps'. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0536ccb29551400ce59713017c86eb5e858aeb5f > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69395/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > test_list_all (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 8 tests in 20.889s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69393: Displayed 'State' field when using 'mesos task list'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69393/#review210660 --- src/python/cli_new/lib/cli/plugins/task/main.py Lines 125 (patched) <https://reviews.apache.org/r/69393/#comment295384> Let's make the default display "UNKNOWN" instead of just blank. src/python/cli_new/lib/cli/plugins/task/main.py Lines 127 (patched) <https://reviews.apache.org/r/69393/#comment295386> We should use [-1] here instaead of pop(). src/python/cli_new/lib/cli/tests/task.py Line 188 (original), 189 (patched) <https://reviews.apache.org/r/69393/#comment295385> Should now be UNKNOWN Keep in mind though, that this is flakey, because we don't know when the task will transition states. - Kevin Klues On Nov. 19, 2018, 2:11 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69393/ > --- > > (Updated Nov. 19, 2018, 2:11 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9399 > https://issues.apache.org/jira/browse/MESOS-9399 > > > Repository: mesos > > > Description > --- > > Displayed 'State' field when using 'mesos task list'. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > 0536ccb29551400ce59713017c86eb5e858aeb5f > src/python/cli_new/lib/cli/tests/task.py > f033b4a7663279c8a7c49ed86b3acdda45ceb107 > > > Diff: https://reviews.apache.org/r/69393/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-9399) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 20.812s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69374: Updated new CLI test step to use binary created by PyInstaller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69374/#review210661 --- Ship it! I have made the changes listed above locally before committing. - Kevin Klues On Nov. 16, 2018, 9:36 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69374/ > --- > > (Updated Nov. 16, 2018, 9:36 nachm.) > > > Review request for mesos, Benjamin Bannier and Kevin Klues. > > > Bugs: MESOS-9396 > https://issues.apache.org/jira/browse/MESOS-9396 > > > Repository: mesos > > > Description > --- > > The integration tests for the new CLI running while building Mesos now > directly use the binary created during the build. That way we make sure > that the binary created using PyInstaller is usable, which is the > artifact that we want to distribute to users in the future. > > Previously, we were only activating the virtual environment to run the > tests thus the binary created by PyInstaller was never properly tested. > To use the binary created by PyInstaller, we simply update the PATH > before running 'mesos-cli-tests'. > > > Diffs > - > > src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 > src/python/cli_new/tests/CMakeLists.txt > 19119d1d1d640c10ef4ec8e245773920359ccb75 > > > Diff: https://reviews.apache.org/r/69374/diff/1/ > > > Testing > --- > > The main test for this change is to make sure that we do not rely on the > `mesos` in `src/python/cli_new/bin/` anymore. To do so, I have followed these > steps: > > For Autotools: > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ PYTHON_3=python36 ../configure --enable-new-cli > $ make check > $ mv /home/agrillet/mesos/src/python/cli_new/bin/mesos > /home/agrillet/mesos/src/python/cli_new/bin/mesos2 > $ make check > ``` > > Then I put `/home/agrillet/mesos/src/python/cli_new/bin/mesos` back. > > For CMake: > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON_3=python36 > $ cmake --build . --target tests > $ ctest -R CLITests -V > $ mv /home/agrillet/mesos/src/python/cli_new/bin/mesos > /home/agrillet/mesos/src/python/cli_new/bin/mesos2 > $ ctest -R CLITests -V > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69390: Added docs describing how to use the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69390/#review210659 --- docs/cli.md Lines 17 (patched) <https://reviews.apache.org/r/69390/#comment295381> Can you add something like ... ``` For now, the CLi is still under development, and not built as part of a standard mesos distribution. To use it ... ``` docs/cli.md Lines 29 (patched) <https://reviews.apache.org/r/69390/#comment295382> newline after this? docs/cli.md Lines 54 (patched) <https://reviews.apache.org/r/69390/#comment295383> Let's move these instructions up somehwere near where I mentioned that its still under development, etc. - Kevin Klues On Nov. 19, 2018, 10:31 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69390/ > --- > > (Updated Nov. 19, 2018, 10:31 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9333 > https://issues.apache.org/jira/browse/MESOS-9333 > > > Repository: mesos > > > Description > --- > > The documentation describes the main commands of the CLI, how to enable > it and how to build Mesos including this component. > > > Diffs > - > > docs/cli.md PRE-CREATION > docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 > > > Diff: https://reviews.apache.org/r/69390/diff/1/ > > > Testing > --- > > Checked the rendered Markdown code. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69381: Updated configuration docs describing how to build the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69381/#review210658 --- Ship it! Ship It! - Kevin Klues On Nov. 18, 2018, 5:58 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69381/ > --- > > (Updated Nov. 18, 2018, 5:58 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9333 > https://issues.apache.org/jira/browse/MESOS-9333 > > > Repository: mesos > > > Description > --- > > Updated configuration docs describing how to build the new CLI. > > > Diffs > - > > docs/configuration/autotools.md 3c3c59a6343e00df16f4d0698fd61fb5ef792c1c > docs/configuration/cmake.md 0cba405c53c5e2efdc7170abdafdd4a5db331804 > > > Diff: https://reviews.apache.org/r/69381/diff/1/ > > > Testing > --- > > Checked the rendered Markdown code. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69380: Added configuration docs describing how to use Python 3.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69380/#review210657 --- Ship it! Ship It! - Kevin Klues On Nov. 18, 2018, 5:58 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69380/ > --- > > (Updated Nov. 18, 2018, 5:58 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9333 > https://issues.apache.org/jira/browse/MESOS-9333 > > > Repository: mesos > > > Description > --- > > For Autotools, this means how to use 'PYTHON_3' and 'PYTHON_3_VERSION'. > > For CMake, this means how to use '-DPYTHON_3'. > > > Diffs > - > > docs/configuration/autotools.md 3c3c59a6343e00df16f4d0698fd61fb5ef792c1c > docs/configuration/cmake.md 0cba405c53c5e2efdc7170abdafdd4a5db331804 > > > Diff: https://reviews.apache.org/r/69380/diff/1/ > > > Testing > --- > > Checked the rendered Markdown code. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69374: Updated new CLI test step to use binary created by PyInstaller.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69374/#review210655
---
src/Makefile.am
Lines 2827 (patched)
<https://reviews.apache.org/r/69374/#comment295379>
Since we are doing the postactivate, we don't need the full path to
mesos-cli-tests anymore, let's remove it...
src/python/cli_new/tests/CMakeLists.txt
Lines 26 (patched)
<https://reviews.apache.org/r/69374/#comment295380>
Ditto
- Kevin Klues
On Nov. 16, 2018, 9:36 nachm., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69374/
> ---
>
> (Updated Nov. 16, 2018, 9:36 nachm.)
>
>
> Review request for mesos, Benjamin Bannier and Kevin Klues.
>
>
> Bugs: MESOS-9396
> https://issues.apache.org/jira/browse/MESOS-9396
>
>
> Repository: mesos
>
>
> Description
> ---
>
> The integration tests for the new CLI running while building Mesos now
> directly use the binary created during the build. That way we make sure
> that the binary created using PyInstaller is usable, which is the
> artifact that we want to distribute to users in the future.
>
> Previously, we were only activating the virtual environment to run the
> tests thus the binary created by PyInstaller was never properly tested.
> To use the binary created by PyInstaller, we simply update the PATH
> before running 'mesos-cli-tests'.
>
>
> Diffs
> -
>
> src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190
> src/python/cli_new/tests/CMakeLists.txt
> 19119d1d1d640c10ef4ec8e245773920359ccb75
>
>
> Diff: https://reviews.apache.org/r/69374/diff/1/
>
>
> Testing
> ---
>
> The main test for this change is to make sure that we do not rely on the
> `mesos` in `src/python/cli_new/bin/` anymore. To do so, I have followed these
> steps:
>
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli
> $ make check
> $ mv /home/agrillet/mesos/src/python/cli_new/bin/mesos
> /home/agrillet/mesos/src/python/cli_new/bin/mesos2
> $ make check
> ```
>
> Then I put `/home/agrillet/mesos/src/python/cli_new/bin/mesos` back.
>
> For CMake:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON_3=python36
> $ cmake --build . --target tests
> $ ctest -R CLITests -V
> $ mv /home/agrillet/mesos/src/python/cli_new/bin/mesos
> /home/agrillet/mesos/src/python/cli_new/bin/mesos2
> $ ctest -R CLITests -V
> ```
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 69208: Updated new CLI task attach/exec exit strategy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69208/#review210308 --- Ship it! Ship It! - Kevin Klues On Okt. 30, 2018, 12:55 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69208/ > --- > > (Updated Okt. 30, 2018, 12:55 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9363 > https://issues.apache.org/jira/browse/MESOS-9363 > > > Repository: mesos > > > Description > --- > > Updated new CLI task attach/exec exit strategy. > > > Diffs > - > > src/python/cli_new/lib/cli/mesos.py > 4a5777bf712c3e5fd20a2bfd6da8fb9f3975d120 > src/python/cli_new/lib/cli/plugins/task/main.py > 5866a23aa0c0dfe141fd8fbe4c9ccd04a2c13a08 > src/python/cli_new/lib/cli/tests/task.py > ce3a55d4b66ca74b6be892d058644281ee5c803d > > > Diff: https://reviews.apache.org/r/69208/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (exec-attach) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 17.238s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69206: Updated new CLI commands for new CLI to return proper exit status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69206/#review210305 --- Ship it! Ship It! - Kevin Klues On Okt. 30, 2018, 12:51 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69206/ > --- > > (Updated Okt. 30, 2018, 12:51 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9363 > https://issues.apache.org/jira/browse/MESOS-9363 > > > Repository: mesos > > > Description > --- > > In a later commit, this will be used by the two subcommands 'task > attach' and 'task exec' to return their proper exit sequences. > > > Diffs > - > > src/python/cli_new/bin/main.py 27783caf4eab6ea18eee5a7b90b0dd368c8b4020 > src/python/cli_new/lib/cli/plugins/base.py > c85abd6755c4b046ab4ecbda825f3a1615fb4ce5 > > > Diff: https://reviews.apache.org/r/69206/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (exec-attach) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 17.238s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69237: Simplified newline handling in 'test_exec()' test for new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69237/#review210303 --- Ship it! Ship It! - Kevin Klues On Nov. 1, 2018, 10:28 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69237/ > --- > > (Updated Nov. 1, 2018, 10:28 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9343 > https://issues.apache.org/jira/browse/MESOS-9343 > > > Repository: mesos > > > Description > --- > > The test was previously using '.strip()' to compare the command stdout > and the result we expect. By replacing the task command to use 'printf' > instead of 'echo', we are able to simplify this assertion. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/task.py > ce3a55d4b66ca74b6be892d058644281ee5c803d > > > Diff: https://reviews.apache.org/r/69237/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (exec-attach) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 17.238s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69207: Moved 'updated_tasks()' to new CLI tests base.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69207/#review210302 --- Ship it! I reworked this commit slightly before pushing it in order to keep the test code cleaner. - Kevin Klues On Okt. 30, 2018, 12:53 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69207/ > --- > > (Updated Okt. 30, 2018, 12:53 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9343 > https://issues.apache.org/jira/browse/MESOS-9343 > > > Repository: mesos > > > Description > --- > > Moved 'updated_tasks()' to new CLI tests base. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 7fd39af665164eeef8bb1359de340ef923dc2272 > src/python/cli_new/lib/cli/tests/task.py > ce3a55d4b66ca74b6be892d058644281ee5c803d > > > Diff: https://reviews.apache.org/r/69207/diff/3/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (exec-attach) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 17.238s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69207: Moved 'updated_tasks()' to new CLI tests base.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69207/#review210300 --- src/python/cli_new/lib/cli/tests/base.py Lines 507 (patched) <https://reviews.apache.org/r/69207/#comment294957> As a helper, calling this `updated_tasks()` is weird. Let's call it `running_tasks()`. - Kevin Klues On Okt. 30, 2018, 12:53 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69207/ > --- > > (Updated Okt. 30, 2018, 12:53 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9343 > https://issues.apache.org/jira/browse/MESOS-9343 > > > Repository: mesos > > > Description > --- > > Moved 'updated_tasks()' to new CLI tests base. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 7fd39af665164eeef8bb1359de340ef923dc2272 > src/python/cli_new/lib/cli/tests/task.py > ce3a55d4b66ca74b6be892d058644281ee5c803d > > > Diff: https://reviews.apache.org/r/69207/diff/3/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (exec-attach) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 17.238s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69116: Added 'popen_tty()' to test util functions for the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69116/#review210299 --- Ship it! Ship It! - Kevin Klues On Okt. 22, 2018, 12:23 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69116/ > --- > > (Updated Okt. 22, 2018, 12:23 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9343 > https://issues.apache.org/jira/browse/MESOS-9343 > > > Repository: mesos > > > Description > --- > > This code was pulled directly from: > https://github.com/dcos/dcos-core-cli/blob/ > 7fd55421939a7782c237e2b8719c0fe2f543acd7/ > python/lib/dcoscli/dcoscli/test/common.py > > This function will be used by tests requiring a TTY. This will be the > case for tests concerning the 'task attach' subcommand. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 7fd39af665164eeef8bb1359de340ef923dc2272 > > > Diff: https://reviews.apache.org/r/69116/diff/3/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (exec-attach) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 7 tests in 17.238s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69115: Added test for interactive 'task exec'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69115/#review210266 --- Committed with changes from comments applied by me manually. - Kevin Klues On Okt. 22, 2018, 2:40 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69115/ > --- > > (Updated Okt. 22, 2018, 2:40 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9342 > https://issues.apache.org/jira/browse/MESOS-9342 > > > Repository: mesos > > > Description > --- > > Added test for interactive 'task exec'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/task.py > 4b03cf13185f1c718f24a990a75a4dd74e7e132a > > > Diff: https://reviews.apache.org/r/69115/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 6 tests in 12.602s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69115: Added test for interactive 'task exec'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69115/#review210265 --- Ship it! Ship It! - Kevin Klues On Okt. 22, 2018, 2:40 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69115/ > --- > > (Updated Okt. 22, 2018, 2:40 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9342 > https://issues.apache.org/jira/browse/MESOS-9342 > > > Repository: mesos > > > Description > --- > > Added test for interactive 'task exec'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/task.py > 4b03cf13185f1c718f24a990a75a4dd74e7e132a > > > Diff: https://reviews.apache.org/r/69115/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 6 tests in 12.602s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69115: Added test for interactive 'task exec'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69115/#review210242 --- src/python/cli_new/lib/cli/tests/task.py Lines 95 (patched) <https://reviews.apache.org/r/69115/#comment294870> Typo src/python/cli_new/lib/cli/tests/task.py Lines 126 (patched) <https://reviews.apache.org/r/69115/#comment294871> Why do we need the strip() here? - Kevin Klues On Okt. 22, 2018, 2:40 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69115/ > --- > > (Updated Okt. 22, 2018, 2:40 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9342 > https://issues.apache.org/jira/browse/MESOS-9342 > > > Repository: mesos > > > Description > --- > > Added test for interactive 'task exec'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/task.py > 4b03cf13185f1c718f24a990a75a4dd74e7e132a > > > Diff: https://reviews.apache.org/r/69115/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 6 tests in 12.602s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69115: Added test for interactive 'task exec'.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69115/#review209864
---
src/python/cli_new/lib/cli/tests/task.py
Lines 113 (patched)
<https://reviews.apache.org/r/69115/#comment294467>
Can we add the same as for the non-interactive case:
```
raise CLIException("Unable to find running task in master state
'{master}'".format(master=master))
```
src/python/cli_new/lib/cli/tests/task.py
Lines 117-118 (patched)
<https://reviews.apache.org/r/69115/#comment294468>
Need to change this based on recent constants added for TEST_DIRECTORY and
TEST_DATA_DIRECTORY
src/python/cli_new/lib/cli/tests/task.py
Lines 120 (patched)
<https://reviews.apache.org/r/69115/#comment294469>
This comment seems unnecessary
src/python/cli_new/lib/cli/tests/task.py
Lines 124-126 (patched)
<https://reviews.apache.org/r/69115/#comment294470>
I don't really like the way we do this. I'd rather just open it twice and
get the content twice.
- Kevin Klues
On Okt. 22, 2018, 11:59 vorm., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69115/
> ---
>
> (Updated Okt. 22, 2018, 11:59 vorm.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-9342
> https://issues.apache.org/jira/browse/MESOS-9342
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added test for interactive 'task exec'.
>
>
> Diffs
> -
>
> src/python/cli_new/lib/cli/tests/task.py
> b54ade557f579a489e459f6022807146e0211fb0
>
>
> Diff: https://reviews.apache.org/r/69115/diff/1/
>
>
> Testing
> ---
>
> ```
> (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests
> Running the Mesos CLI unit tests
>
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
>
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
>
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
>
> --
> Ran 6 tests in 12.602s
>
> OK
> ```
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 69049: Added test for 'task exec'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69049/#review209862 --- Ship it! Ship It! - Kevin Klues On Okt. 22, 2018, 2:13 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69049/ > --- > > (Updated Okt. 22, 2018, 2:13 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > Added test for 'task exec'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION > src/python/cli_new/lib/cli/tests/task.py > b54ade557f579a489e459f6022807146e0211fb0 > > > Diff: https://reviews.apache.org/r/69049/diff/3/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 5 tests in 8.754s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69119: Added new CLI constants 'TEST_DIRECTORY' and 'TEST_DATA_DIRECTORY'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69119/#review209860 --- Ship it! Ship It! - Kevin Klues On Okt. 22, 2018, 2:12 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69119/ > --- > > (Updated Okt. 22, 2018, 2:12 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > Added new CLI constants 'TEST_DIRECTORY' and 'TEST_DATA_DIRECTORY'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/constants.py > 1398701b167eae397730afc5b1fab6a21e723266 > > > Diff: https://reviews.apache.org/r/69119/diff/1/ > > > Testing > --- > > > Thanks, > > Armand Grillet > >
Re: Review Request 69049: Added test for 'task exec'.
> On Okt. 22, 2018, 1:45 nachm., Kevin Klues wrote: > > src/python/cli_new/lib/cli/tests/task.py > > Lines 41 (patched) > > <https://reviews.apache.org/r/69049/diff/1/?file=2101777#file2101777line41> > > > > It seems strange to just assume we are in the correct directory when > > this is ran since this is just a relative path. Does this work if we run > > the tests from elsewhere? We can probably just add a nother path element to the front of this with: `os.path.dirname(os.path.realpath(__file__))` What I don't understand yet though is how this is working until now. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69049/#review209858 --- On Okt. 22, 2018, 1:41 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69049/ > --- > > (Updated Okt. 22, 2018, 1:41 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > Added test for 'task exec'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION > src/python/cli_new/lib/cli/tests/task.py > b54ade557f579a489e459f6022807146e0211fb0 > > > Diff: https://reviews.apache.org/r/69049/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 5 tests in 8.754s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69049: Added test for 'task exec'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69049/#review209858 --- src/python/cli_new/lib/cli/tests/data/task/lorem-ipsum.txt Lines 1 (patched) <https://reviews.apache.org/r/69049/#comment294459> Let's put this one level up (i.e. not in a nested `task` directory). src/python/cli_new/lib/cli/tests/task.py Lines 41 (patched) <https://reviews.apache.org/r/69049/#comment294460> It seems strange to just assume we are in the correct directory when this is ran since this is just a relative path. Does this work if we run the tests from elsewhere? src/python/cli_new/lib/cli/tests/task.py Lines 58-64 (patched) <https://reviews.apache.org/r/69049/#comment294461> This seems really messy at first glance. Is there some way to clean this up so it's easier to read? Maybe just a comment on what we are doing here will suffice. src/python/cli_new/lib/cli/tests/task.py Lines 74-75 (patched) <https://reviews.apache.org/r/69049/#comment294462> What is going on here? Why are we actually using a bare-raise here? src/python/cli_new/lib/cli/tests/task.py Lines 82 (patched) <https://reviews.apache.org/r/69049/#comment294463> We shouldn't have to decode here anymore after moving the `exec_command` to use `universal_newline`. - Kevin Klues On Okt. 22, 2018, 1:41 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69049/ > --- > > (Updated Okt. 22, 2018, 1:41 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > Added test for 'task exec'. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION > src/python/cli_new/lib/cli/tests/task.py > b54ade557f579a489e459f6022807146e0211fb0 > > > Diff: https://reviews.apache.org/r/69049/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_exec (cli.tests.task.TestTaskPlugin) ... ok > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 5 tests in 8.754s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69048: Added tenacity to 'pip-requirements' for new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69048/#review209853 --- Ship it! Ship It! - Kevin Klues On Okt. 22, 2018, 10:31 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69048/ > --- > > (Updated Okt. 22, 2018, 10:31 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > This requirement will be used in upcoming new CLI integration tests. > > > Diffs > - > > src/python/cli_new/pip-requirements.txt > dfbc28fb041b5cdf6bf9440fab0730eab0a4c162 > > > Diff: https://reviews.apache.org/r/69048/diff/1/ > > > Testing > --- > > Tested later in the chain. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69114: Added 'exec_command' to test util functions for the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69114/#review209851 --- Ship it! Ship It! - Kevin Klues On Okt. 22, 2018, 1:26 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69114/ > --- > > (Updated Okt. 22, 2018, 1:26 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > This code was mostly pulled directly from: > https://github.com/dcos/dcos-core-cli/blob/ > 7fd55421939a7782c237e2b8719c0fe2f543acd7/ > python/lib/dcoscli/dcoscli/test/common.py > > This function will be used by tests that do not return a specific output > but an error code, stdout, and stderr. This will be the case for tests > concerning the 'task exec' and 'task attach' subcommands. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 3fb471c1f49e930d908322055bb9a188f88ee602 > > > Diff: https://reviews.apache.org/r/69114/diff/2/ > > > Testing > --- > > Tested later in the chain. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69114: Added 'exec_command' to test util functions for the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69114/#review209850 --- src/python/cli_new/lib/cli/tests/base.py Lines 462 (patched) <https://reviews.apache.org/r/69114/#comment294457> We shouldn't do this. We should raise a CLIException like we do everywhere else. src/python/cli_new/lib/cli/tests/base.py Lines 486 (patched) <https://reviews.apache.org/r/69114/#comment294458> Instead of doing this, we should use the argument to Popen with `universal_newlines=True`. - Kevin Klues On Okt. 22, 2018, 10:31 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69114/ > --- > > (Updated Okt. 22, 2018, 10:31 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9341 > https://issues.apache.org/jira/browse/MESOS-9341 > > > Repository: mesos > > > Description > --- > > This code was pulled directly from: > https://github.com/dcos/dcos-core-cli/blob/ > 7fd55421939a7782c237e2b8719c0fe2f543acd7/ > python/lib/dcoscli/dcoscli/test/common.py > > This function will be used by tests that do not return a specific output > but an error code, stdout, and stderr. This will be the case for tests > concerning the 'task exec' and 'task attach' subcommands. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 3fb471c1f49e930d908322055bb9a188f88ee602 > > > Diff: https://reviews.apache.org/r/69114/diff/1/ > > > Testing > --- > > Tested later in the chain. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69105: Added 'task attach' subcommand to new CLI.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69105/#review209823
---
Ship it!
Ship It!
- Kevin Klues
On Okt. 20, 2018, 10:04 vorm., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69105/
> ---
>
> (Updated Okt. 20, 2018, 10:04 vorm.)
>
>
> Review request for mesos and Kevin Klues.
>
>
> Bugs: MESOS-6551
> https://issues.apache.org/jira/browse/MESOS-6551
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added 'task attach' subcommand to new CLI.
>
>
> Diffs
> -
>
> src/python/cli_new/lib/cli/plugins/task/main.py
> 546ad00ee88fa51c54520a94da19752d010004a8
>
>
> Diff: https://reviews.apache.org/r/69105/diff/2/
>
>
> Testing
> ---
>
> ```
> $ sudo bin/mesos-master.sh \
> --ip=127.0.0.1 \
> --work_dir=/var/lib/mesos
>
>
> $ sudo bin/mesos-agent.sh
> --master=127.0.0.1:5050 \
> --work_dir=/var/lib/mesos \
> --image_providers=docker \
> --executor_environment_variables="{}" \
> --isolation="cgroups/all,filesystem/linux,docker/runtime,namespaces/pid"
>
>
>
> $ sudo src/mesos-execute \
> --master=127.0.0.1:5050 \
> --name=tty-test \
> --docker_image=library/alpine \
> --no-shell \
> --tty \
> --command="sh -i"
> $ cd src/python/cli_new
> $ source activate
> $ mesos task attach tty-test
> sh-4.3$
> ```
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 69103: Fixed bug in 'execute.cpp' with tty-based tasks and no 'containerInfo'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69103/ --- (Updated Okt. 20, 2018, 9:52 vorm.) Review request for mesos, Armand Grillet, Gilbert Song, and Jie Yu. Bugs: MESOS-8978 https://issues.apache.org/jira/browse/MESOS-8978 Repository: mesos Description --- Previously, we could only launch tasks using the '--tty' flag if they had a backing docker image (or some other combination of other flags set that would cause the task to have a 'containerInfo' created for it). This commit makes sure that if '--tty' is passed, that a containerInfo gets created and its TTYInfo field gets populated. Diffs - src/cli/execute.cpp eb5c5646ed84533d39ad2588c78f1c0caa5fccfb Diff: https://reviews.apache.org/r/69103/diff/1/ Testing (updated) --- sudo bin/mesos-master.sh \ --ip=127.0.0.1 \ --work_dir=/var/lib/mesos sudo bin/mesos-agent.sh --master=127.0.0.1:5050 \ --work_dir=/var/lib/mesos sudo src/mesos-execute \ --master=127.0.0.1:5050 \ --name=tty-test \ --tty \ --command="sh -i" $ mesos task attach tty-test sh-4.3$ Thanks, Kevin Klues
Re: Review Request 69104: Fixed formatting in subcommand help in the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69104/ --- (Updated Okt. 20, 2018, 9:37 vorm.) Review request for mesos and Armand Grillet. Changes --- Updated testing done Repository: mesos Description --- Previously, there was some weird formatting with extra newlines when providing multi-line 'long_help' strings or extra flags to the subcommands. This commit fixes that. Diffs - src/python/cli_new/lib/cli/util.py 6a012369fa151ffd6b73a071342073a55ef9cc6b Diff: https://reviews.apache.org/r/69104/diff/1/ Testing (updated) --- Old Output: ``` Attach the CLI to the stdio of a running task Usage: mesos task attach (-h | --help) mesos task attach --version mesos task attach [options] Options: --no-stdin do not attach a stdin [default: False] -h --help Show this screen. Description: Attach the CLI to the stdio of a running task To detach type the sequence CTRL-p CTRL-q. ``` New output: ``` Attach the CLI to the stdio of a running task Usage: mesos task attach (-h | --help) mesos task attach --version mesos task attach [options] Options: --no-stdin do not attach a stdin [default: False] -h --help Show this screen. Description: Attach the CLI to the stdio of a running task To detach type the sequence CTRL-p CTRL-q. ``` Thanks, Kevin Klues
Review Request 69104: Fixed formatting in subcommand help in the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69104/ --- Review request for mesos and Armand Grillet. Repository: mesos Description --- Previously, there was some weird formatting with extra newlines when providing multi-line 'long_help' strings or extra flags to the subcommands. This commit fixes that. Diffs - src/python/cli_new/lib/cli/util.py 6a012369fa151ffd6b73a071342073a55ef9cc6b Diff: https://reviews.apache.org/r/69104/diff/1/ Testing --- Thanks, Kevin Klues
Review Request 69103: Fixed bug in 'execute.cpp' with tty-based tasks and no 'containerInfo'.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69103/ --- Review request for mesos, Armand Grillet, Gilbert Song, and Jie Yu. Bugs: MESOS-8978 https://issues.apache.org/jira/browse/MESOS-8978 Repository: mesos Description --- Previously, we could only launch tasks using the '--tty' flag if they had a backing docker image (or some other combination of other flags set that would cause the task to have a 'containerInfo' created for it). This commit makes sure that if '--tty' is passed, that a containerInfo gets created and its TTYInfo field gets populated. Diffs - src/cli/execute.cpp eb5c5646ed84533d39ad2588c78f1c0caa5fccfb Diff: https://reviews.apache.org/r/69103/diff/1/ Testing --- sudo bin/mesos-master.sh \ --ip=127.0.0.1 \ --work_dir=/var/lib/mesos sudo bin/mesos-agent.sh --master=127.0.0.1:5050 \ --work_dir=/var/lib/mesos sudo src/mesos-execute \ --master=127.0.0.1:5050 \ --name=tty-test \ --tty \ --command="sh -i" Thanks, Kevin Klues
Re: Review Request 69003: Added `task exec` to new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69003/#review209750 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 6:53 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69003/ > --- > > (Updated Okt. 15, 2018, 6:53 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > Added `task exec` to new CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > a47a8c53ee2d8d468ea0f9947ea3f65d81fc9251 > > > Diff: https://reviews.apache.org/r/69003/diff/4/ > > > Testing > --- > > Only tested manually at the moment. > ``` > $ mesos task exec --interactive --tty vi > $ mesos task exec -it bash > $ mesos task exec cat stderr > $ echo "hello" | mesos task exec --interactive cat > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69003: Added `task exec` to new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69003/#review209749 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 6:53 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69003/ > --- > > (Updated Okt. 15, 2018, 6:53 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > Added `task exec` to new CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > a47a8c53ee2d8d468ea0f9947ea3f65d81fc9251 > > > Diff: https://reviews.apache.org/r/69003/diff/4/ > > > Testing > --- > > Only tested manually at the moment. > ``` > $ mesos task exec --interactive --tty vi > $ mesos task exec -it bash > $ mesos task exec cat stderr > $ echo "hello" | mesos task exec --interactive cat > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69003: Added `task exec` to new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69003/#review209748 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 6:53 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69003/ > --- > > (Updated Okt. 15, 2018, 6:53 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > Added `task exec` to new CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > a47a8c53ee2d8d468ea0f9947ea3f65d81fc9251 > > > Diff: https://reviews.apache.org/r/69003/diff/4/ > > > Testing > --- > > Only tested manually at the moment. > ``` > $ mesos task exec --interactive --tty vi > $ mesos task exec -it bash > $ mesos task exec cat stderr > $ echo "hello" | mesos task exec --interactive cat > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 68978: Added TaskIO object to new CLI for `task exec` and `task attach`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68978/#review209747 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 11:06 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68978/ > --- > > (Updated Okt. 15, 2018, 11:06 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > Added TaskIO object to new CLI for `task exec` and `task attach`. > > > Diffs > - > > src/python/cli_new/lib/cli/mesos.py > 7cf84bcf1d327bc9c63934e371692cef989ad3aa > > > Diff: https://reviews.apache.org/r/68978/diff/5/ > > > Testing > --- > > > Thanks, > > Armand Grillet > >
Re: Review Request 69023: Added `retry` argument to `request()` method in Python library.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69023/#review209693 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 11:05 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69023/ > --- > > (Updated Okt. 15, 2018, 11:05 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > This allows us to do HTTP requests without using `tenacity.retry()`. We > need to be able to disable these retries to stream data for a long > period of time, such as when using the upcoming `mesos task exec`. > > > Diffs > - > > src/python/lib/mesos/http.py cd1587797db7d75c6b839851f0f3e5671269307c > > > Diff: https://reviews.apache.org/r/69023/diff/2/ > > > Testing > --- > > Used `task exec` successfully later in this chain. Tests suite of the lib > still passing. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69007: Added `get_container_id` to util functions for the new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69007/#review209692 --- Ship it! Ship It! - Kevin Klues On Okt. 13, 2018, 9:08 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69007/ > --- > > (Updated Okt. 13, 2018, 9:08 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > This function will be used by the `TaskIO` object to know which > container to use in order to run `task attach` and `task exec.`. > > > Diffs > - > > src/python/cli_new/lib/cli/mesos.py > 26556c59fffc5b069549b1fd318b104884d52bf0 > > > Diff: https://reviews.apache.org/r/69007/diff/2/ > > > Testing > --- > > > Thanks, > > Armand Grillet > >
Re: Review Request 68971: Moved import of '../lib' from new CLI bootstrap to pip-requirements.txt.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68971/#review209615 --- Ship it! Ship It! - Kevin Klues On Okt. 16, 2018, 11:33 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68971/ > --- > > (Updated Okt. 16, 2018, 11:33 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8795 > https://issues.apache.org/jira/browse/MESOS-8795 > > > Repository: mesos > > > Description > --- > > This adds the Mesos Python package to the requirements needed by the > new CLI. We were previously adding the path to the package when setting > up the CLI virtual environment but this does not work with Pylint when > it is run from the linters virtual environment. > > By setting things in `pip-requirements.txt`, Pylint does not report > inexisting linting errors when doing imports such as `import mesos`. > > > Diffs > - > > src/python/cli_new/bootstrap a6183d4c28281bf6d29c8b7f825ae474056f027b > src/python/cli_new/pip-requirements.txt > d1822bf752ce76aa5da5999057fe1efb83747fd0 > > > Diff: https://reviews.apache.org/r/68971/diff/3/ > > > Testing > --- > > Updated the agent plugin to do `from mesos.exceptions import MesosException` > and use `MesosException`. Commited to see the git hook running and saw the > error `E0401: Unable to import 'mesos.exceptions' (import-error).`. Updated > `support/pylint.config`, run the git hook again and saw that the error was > gone. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69047: Updated Python library to be easier to handle as a Python module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69047/#review209612 --- Ship it! Ship It! - Kevin Klues On Okt. 16, 2018, 11:31 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69047/ > --- > > (Updated Okt. 16, 2018, 11:31 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8795 > https://issues.apache.org/jira/browse/MESOS-8795 > > > Repository: mesos > > > Description > --- > > Updated Python library to be easier to handle as a Python module. > > > Diffs > - > > src/python/lib/mesos/__init__.py 40219c5e03915754b61fc5d7263a063f0d18cca1 > src/python/lib/setup.py 08f854f43681d1f694bb48604773256be7ce927b > > > Diff: https://reviews.apache.org/r/69047/diff/1/ > > > Testing > --- > > Tested manually using the Mesos CLI later in the chain. > > > Thanks, > > Armand Grillet > >
Re: Review Request 69026: Changed usage documentation for new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69026/#review209536 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 11:45 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69026/ > --- > > (Updated Okt. 15, 2018, 11:45 vorm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > Moved the `[options]` in the usage documentation > to where they should be set when using the CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/base.py > 6cba828d886dcc2e1d0a00514f62d87634aafe9e > > > Diff: https://reviews.apache.org/r/69026/diff/1/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos task > Interacts with the tasks running in a Mesos cluster > > Usage: > mesos task (-h | --help) > mesos task --version > mesos task (-h | --help) > mesos task [options] [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > exec Execute commands in a task's container > list List all active tasks in a Mesos cluster > ``` > And: > ``` > (mesos-cli) ? cli_new (MESOS-6551) ? mesos task exec > Execute commands in a task's container > > Usage: > mesos task exec (-h | --help) > mesos task exec --version > mesos task exec [options] [...] > > Options: > -h --help Show this screen. > -i --interactive interactive [default: False] > -t --tty tty [default: False] > > > Description: > Execute commands in a task's container > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 69003: Added `task exec` to new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69003/#review209526 --- src/python/cli_new/lib/cli/plugins/task/main.py Lines 60-62 (patched) <https://reviews.apache.org/r/69003/#comment294016> This comment is incorrect. src/python/cli_new/lib/cli/plugins/task/main.py Lines 70-71 (patched) <https://reviews.apache.org/r/69003/#comment294017> We usually either put all arguments on one line, or just one per line. - Kevin Klues On Okt. 12, 2018, 9:18 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69003/ > --- > > (Updated Okt. 12, 2018, 9:18 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-6551 > https://issues.apache.org/jira/browse/MESOS-6551 > > > Repository: mesos > > > Description > --- > > Added `task exec` to new CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/plugins/task/main.py > a47a8c53ee2d8d468ea0f9947ea3f65d81fc9251 > > > Diff: https://reviews.apache.org/r/69003/diff/3/ > > > Testing > --- > > Only tested manually at the moment. > ``` > $ mesos task exec --interactive --tty vi > $ mesos task exec -it bash > $ mesos task exec cat stderr > $ echo "hello" | mesos task exec --interactive cat > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 68965: Added try/catch statements when using Mesos util functions in new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68965/#review209367 --- Ship it! Ship It! - Kevin Klues On Okt. 9, 2018, 2:18 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68965/ > --- > > (Updated Okt. 9, 2018, 2:18 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8795 > https://issues.apache.org/jira/browse/MESOS-8795 > > > Repository: mesos > > > Description > --- > > Added try/catch statements when using Mesos util functions in new CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/mesos.py > 7cf84bcf1d327bc9c63934e371692cef989ad3aa > src/python/cli_new/lib/cli/plugins/agent/main.py > 5e821b36162cea8183f233a86181144daeca0753 > src/python/cli_new/lib/cli/plugins/task/main.py > a47a8c53ee2d8d468ea0f9947ea3f65d81fc9251 > > > Diff: https://reviews.apache.org/r/68965/diff/2/ > > > Testing > --- > > ``` > (mesos-cli) ? cli_new (MESOS-8795) ? mesos-cli-tests > Running the Mesos CLI unit tests > > TestAgentPlugin > test_list (cli.tests.agent.TestAgentPlugin) ... ok > > TestInfrastructure > test_capture_output (cli.tests.tests.TestInfrastructure) ... ok > test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok > > TestTaskPlugin > test_list (cli.tests.task.TestTaskPlugin) ... ok > > -- > Ran 4 tests in 5.350s > > OK > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 66683: Updated address sanitization in new CLI to accept DNS names.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66683/#review209358 --- Ship it! Ship It! - Kevin Klues On Okt. 9, 2018, 12:49 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66683/ > --- > > (Updated Okt. 9, 2018, 12:49 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8025 > https://issues.apache.org/jira/browse/MESOS-8025 > > > Repository: mesos > > > Description > --- > > Updated address sanitization in new CLI to accept DNS names. > > > Diffs > - > > src/python/cli_new/lib/cli/config.py > c88952b9f87f1cf558d7aef44803d7a24873d8bf > src/python/cli_new/lib/cli/http.py d1faac19cbe2b52ef941053ec6e2e52a1bfcd3db > src/python/cli_new/lib/cli/util.py e79268d57bc9be4514e8f087060ea971e9b09c3a > > > Diff: https://reviews.apache.org/r/66683/diff/7/ > > > Testing > --- > > Ran `mesos agent list` successfully with config: > ``` > [master] > address = "http://.com:5061" > ``` > On the server providing `http://.com` I had > a Mesos master running with `bash mesos-master.sh --port='5061' > --work_dir='/tmp/master1' --log_dir='/tmp/master1-log' --registry=in_memory`. > > Also ran `mesos-cli-tests` successfully. > > > Thanks, > > Armand Grillet > >
Re: Review Request 68950: Moved `get_agent_address` from `util.py` to `mesos.py` in new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68950/#review209309 --- Ship it! Ship It! - Kevin Klues On Okt. 8, 2018, 1:08 nachm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68950/ > --- > > (Updated Okt. 8, 2018, 1:08 nachm.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8795 > https://issues.apache.org/jira/browse/MESOS-8795 > > > Repository: mesos > > > Description > --- > > Moved `get_agent_address` from `util.py` to `mesos.py` in new CLI. > > > Diffs > - > > src/python/cli_new/lib/cli/mesos.py > 068d694fb6407565003c9b65a71c2b1f7fb70c77 > src/python/cli_new/lib/cli/util.py 7cec7e49ab3c0926067626f5bd5dbba8bf4f44d7 > > > Diff: https://reviews.apache.org/r/68950/diff/1/ > > > Testing > --- > > ``` > $ mesos-cli-tests > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 68734: Refactored new CLI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68734/#review209148 --- Ship it! Armand and I sat together and modified this RR. We decided to move the functionality spread across two files (tasks.py and agents.py) into a single mesos.py that has functions to abstract the various Mesos API calls. The main logic remains the same, it is just rearranged. - Kevin Klues On Sept. 17, 2018, 10:36 vorm., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68734/ > --- > > (Updated Sept. 17, 2018, 10:36 vorm.) > > > Review request for mesos, Eric Chung and Kevin Klues. > > > Bugs: MESOS-8795 > https://issues.apache.org/jira/browse/MESOS-8795 > > > Repository: mesos > > > Description > --- > > This adds two new files, `agent.py` and `tasks.py`, usable by > any plugin to do standard operations with agents and tasks. > > > Diffs > - > > src/python/cli_new/lib/cli/agents.py PRE-CREATION > src/python/cli_new/lib/cli/plugins/agent/main.py > fc62d9fe62001e31d254a038cd7056751c846541 > src/python/cli_new/lib/cli/plugins/task/main.py > 644e256ce898784ce3511c30a373595ace149db9 > src/python/cli_new/lib/cli/tasks.py PRE-CREATION > > > Diff: https://reviews.apache.org/r/68734/diff/1/ > > > Testing > --- > > Ran `mesos-cli-tests` successfully. > > > Thanks, > > Armand Grillet > >
Re: Review Request 68810: Removed call to 'setsid()' in command executor if TTY attached.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68810/ --- (Updated Sept. 29, 2018, 12:05 nachm.) Review request for mesos, Gilbert Song and Jie Yu. Changes --- Addressed @jieyu's comments. Bugs: MESOS-8978 https://issues.apache.org/jira/browse/MESOS-8978 Repository: mesos Description (updated) --- Previously, we would unconditionally call 'setsid()' in the command executor whenever we launched a process. This causes the process it launches to start a new session and NOT inherit the controlling TTY from it's parent. This obviously causes problems, if the goal of attaching a TTY to a task is so that we can actually give it control of that TTY while it is executing. Interestingly, even if process does not control its TTY, it can still read and write from the file descriptors associated with it (it just can't receive any signals associated with it, such as SIGWINCH, SIGHUP, etc.). This is why things "appeared" to mostly work until this point because stdin/stdout/stderr were all being redirected properly to it. Where we saw problems was with trying to 'attach' to an already running process launched via the command executor using the ATTACH_NESTED_CONTAINER_INPUT/OUTPUT calls. If you ran something like 'bash', you would not be able to do job control, and you could not resize the screen properly when running things like 'vim'. This commit fixes this issue by checking to see if a TTY has been attached to a task launched by the command executor, and skipping the 'setsid()' call when forking it. We considered a number of alternative approaches, but finally settled on this one since it was the least invasive. I.e. only tasks launched with a TTY (which is a failry new concept in Mesos) will be affected by this change; the semantics of all traditional tasks launched via the command executor will go unchanged. Note: this problem does not exists for the default executor because the agent does the job of launching all containers, and there is no executor sitting between the containerizer and the actual process of a task intervening to call an extra 'setsid()'. This is why containers launched via LAUNCH_NESTED_CONTAINER_SESSION have always worked as expected. Diffs (updated) - src/launcher/executor.cpp 86fb87dfbaa4daba22ec491ca5fca0e562797521 src/slave/containerizer/mesos/io/switchboard.cpp 498c008c36725b4ed43b4905d3640508beaaae93 Diff: https://reviews.apache.org/r/68810/diff/2/ Changes: https://reviews.apache.org/r/68810/diff/1-2/ Testing --- $ make -j 40 check Thanks, Kevin Klues
Review Request 68810: Removed call to 'setsid()' in command executor if TTY attached.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68810/ --- Review request for mesos, Gilbert Song and Jie Yu. Bugs: MESOS-8978 https://issues.apache.org/jira/browse/MESOS-8978 Repository: mesos Description --- Previously, we would unconditionally call 'setsid()' in the command executor whenever we launched a process. This causes the process it launches to start a new session and NOT inherit the controlling TTY from it's parent. This obviously causes problems, if the goal of attaching a TTY to a task is so that we can actually give it control of that TTY while it is executing. Interestingly, even if process does not control its TTY, it can still read and write from the file descriptors associated with it (it just can't receive any signals associated with it, such as SIGWINCH, NOHUP, etc.). This is why things "appeared" to mostly work until this point because stdin/stdout/stderr were all being redirected properly to it. Where we saw problems was with trying to 'attach' to an already running process launched via the command executor using the ATTACH_NESTED_CONTAINER_INPUT/OUTPUT calls. If you ran something like 'bash', you would not be able to do job control, and you could not resize the screen properly when running things like 'vim'. This commit fixes this issue by checking to see if a TTY has been attached to a tasng launched by the command executor, and skipping the 'setsid()' call when forking it. We considered a number of alternative approaches, but finally settled on this one since it was the least invasive. I.e. only tasks launched with a TTY (which is a failry new concept in Mesos) will be affected by this change; the semantics of all traditional tasks launched via the command executor will go unchanged. Note: this problem does not exists for the default executor because the agent does the job of launching all containers, and there is no executor sitting between the containerizer and the actual process of a task intervening to call an extra 'setsid()'. This is why containers launched via LAUNCH_NESTED_CONTAINER_SESSION have always worked as expected. Diffs - src/launcher/executor.cpp 86fb87dfbaa4daba22ec491ca5fca0e562797521 src/slave/containerizer/mesos/io/switchboard.cpp 52b0e521ed1c651c90b3a3df7c4df576288bf400 Diff: https://reviews.apache.org/r/68810/diff/1/ Testing --- $ make -j 40 check Thanks, Kevin Klues
Re: Review Request 68724: Added the ability to launch tasks with a TTY attached to mesos-execute.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68724/
---
(Updated Sept. 22, 2018, 12:35 vorm.)
Review request for mesos, Gilbert Song and Jie Yu.
Changes
---
Updated "Bugs"section
Bugs: MESOS-8978
https://issues.apache.org/jira/browse/MESOS-8978
Repository: mesos
Description
---
Added the ability to launch tasks with a TTY attached to mesos-execute.
Diffs
-
src/cli/execute.cpp e0a8fc89112223c6b0516ddec0587047338fa264
Diff: https://reviews.apache.org/r/68724/diff/1/
Testing
---
sudo bin/mesos-master.sh \
--ip=127.0.0.1 \
--work_dir=/var/lib/mesos
sudo bin/mesos-agent.sh
--master=127.0.0.1:5050 \
--work_dir=/var/lib/mesos \
--image_providers=docker \
--executor_environment_variables="{}" \
--isolation="cgroups/all,filesystem/linux,docker/runtime,namespaces/pid"
sudo src/mesos-execute \
--master=127.0.0.1:5050 \
--name=tty-test \
--docker_image=library/alpine \
--no-shell \
--tty \
--command="sh -i"
Thanks,
Kevin Klues
Review Request 68724: Added the ability to launch tasks with a TTY attached to mesos-execute.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68724/
---
Review request for mesos, Gilbert Song and Jie Yu.
Repository: mesos
Description
---
Added the ability to launch tasks with a TTY attached to mesos-execute.
Diffs
-
src/cli/execute.cpp e0a8fc89112223c6b0516ddec0587047338fa264
Diff: https://reviews.apache.org/r/68724/diff/1/
Testing
---
sudo bin/mesos-master.sh \
--ip=127.0.0.1 \
--work_dir=/var/lib/mesos
sudo bin/mesos-agent.sh
--master=127.0.0.1:5050 \
--work_dir=/var/lib/mesos \
--image_providers=docker \
--executor_environment_variables="{}" \
--isolation="cgroups/all,filesystem/linux,docker/runtime,namespaces/pid"
sudo src/mesos-execute \
--master=127.0.0.1:5050 \
--name=tty-test \
--docker_image=library/alpine \
--no-shell \
--tty \
--command="sh -i"
Thanks,
Kevin Klues
Re: Review Request 67894: Migrated mesos python package to python3.6.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67894/#review208096 --- Ship it! I manually removed the changes in the tox.ini file since they have been added by armand in an already commited RR. - Kevin Klues On Juli 12, 2018, 8:09 vorm., Eric Chung wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67894/ > --- > > (Updated Juli 12, 2018, 8:09 vorm.) > > > Review request for mesos, Armand Grillet, Jason Lai, Kevin Klues, and Zhitao > Li. > > > Repository: mesos > > > Description > --- > > Migrated mesos python package to python3.6. > > > Diffs > - > > src/python/lib/mesos/exceptions.py 63c7aa632da618d6efb74124d607f5744ca64ce5 > src/python/lib/mesos/http.py 073c159dc6916fa5d7349a033db022d7175aef05 > src/python/lib/tests/conftest.py d4d6efcc45d1266749c5750c3ab47dd742b927c1 > src/python/lib/tests/test_http.py 66dd6d7c6272d1828dd591829ca3543d3430f69b > src/python/lib/tests/test_mesos.py 285a071e3d8ed2b61646264ddccd8bb38df2c0dd > src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db > > > Diff: https://reviews.apache.org/r/67894/diff/1/ > > > Testing > --- > > Ran `tox` under `src/python/lib` > > > Thanks, > > Eric Chung > >
Review Request 68560: Updated the python2 'PyLinter' to only lint python2 based code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68560/ --- Review request for mesos, Armand Grillet and Robin Gögge. Bugs: MESOS-8770 https://issues.apache.org/jira/browse/MESOS-8770 Repository: mesos Description --- Specifically, this includes no longer linting all code under the `src/python` directory. Diffs - support/mesos-style.py 0243eb33b983d0bb94326efbaa19114ec8b08990 Diff: https://reviews.apache.org/r/68560/diff/1/ Testing --- Thanks, Kevin Klues
Re: Review Request 67956: Removed some generic flag parsers that are now in stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67956/#review206247 --- Ship it! I assume removing these in this commit seprate from adding them to stout in the previous commit didn't cause build errors, for the short time between commits when there were duplicate definitions of them? - Kevin Klues On July 18, 2018, 1:19 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67956/ > --- > > (Updated July 18, 2018, 1:19 a.m.) > > > Review request for mesos, Benjamin Mahler and Kevin Klues. > > > Repository: mesos > > > Description > --- > > Removed some generic flag parsers that are now in stout. > > > Diffs > - > > src/common/parse.hpp 03814e3112a043a1001764b316b9d49501d33665 > > > Diff: https://reviews.apache.org/r/67956/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Re: Review Request 67955: Added some new generic flag parsers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67955/#review206242 --- 3rdparty/stout/include/stout/flags/parse.hpp Lines 249-251 (patched) <https://reviews.apache.org/r/67955/#comment289147> This comment is wrong. It's not for unsigned ints, it's for strings. - Kevin Klues On July 18, 2018, 1:19 a.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67955/ > --- > > (Updated July 18, 2018, 1:19 a.m.) > > > Review request for mesos, Benjamin Mahler and Kevin Klues. > > > Repository: mesos > > > Description > --- > > Added some new generic flag parsers. > > > Diffs > - > > 3rdparty/stout/include/stout/flags/parse.hpp > 4566b798b8b66d47779a28cabdea06f588012686 > > > Diff: https://reviews.apache.org/r/67955/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Benjamin Hindman > >
Re: Review Request 67488: Updated CLI to Python 3.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67488/#review206106 --- Ship it! Ship It! - Kevin Klues On July 6, 2018, 1:56 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67488/ > --- > > (Updated July 6, 2018, 1:56 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > The build tools are also up to date thus the CLI can still be built > using Autotools and CMake. No features have been added to the CLI. > > > Diffs > - > > cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 > configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b > src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada > src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca > src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 > src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c > src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f > src/python/cli_new/lib/cli/config.py > 6f92622725d8a042a2a728fd38c977ac690ef6be > src/python/cli_new/lib/cli/docopt.py > 86a4e9c74326fb80cc59487113f07358dd96960d > src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b > src/python/cli_new/lib/cli/plugins/agent/main.py > 59280ece8ebd00bb96df3675b6356a26cc48a2c0 > src/python/cli_new/lib/cli/plugins/task/main.py > cc6cff56c71262729a8870017bef2e97636abe5a > src/python/cli_new/lib/cli/tests/base.py > 89360e6cac5ca910044a5a82fab7237510edee7f > src/python/cli_new/lib/cli/tests/tests.py > 79e1036f6d11c63884091fe43672607b03955c1a > src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 > src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 > src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 > src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db > support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 > support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 > > > Diff: https://reviews.apache.org/r/67488/diff/12/ > > > Testing > --- > > Testing done on Fedora 25 with `python` being Python 2.7, `python3` being > Python 3.5 and `python36` being Python 3.6. > > > For Autotools: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java > $ make check > ``` > > For CMake: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36 > $ cmake --build . -- -j16 > $ ./src/mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > $ cmake --build . --target tests -- -j16 > $ ctest -R CLI > Test project /home/agrillet/apache-mesos/build > Start 4: CLITests > 1/1 Test #4: CLITests . Passed3.63 sec > > 100% tests passed, 0 tests failed out of 1 > ``` > > Checked that the the CLI tests were run, that the content of the directory > build/src/cli was as expected, and that build/src/mesos was correctly running. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67488: Updated CLI to Python 3.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67488/#review206104 --- Ship it! Ship It! - Kevin Klues On July 6, 2018, 1:56 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67488/ > --- > > (Updated July 6, 2018, 1:56 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > The build tools are also up to date thus the CLI can still be built > using Autotools and CMake. No features have been added to the CLI. > > > Diffs > - > > cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 > configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b > src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada > src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca > src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 > src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c > src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f > src/python/cli_new/lib/cli/config.py > 6f92622725d8a042a2a728fd38c977ac690ef6be > src/python/cli_new/lib/cli/docopt.py > 86a4e9c74326fb80cc59487113f07358dd96960d > src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b > src/python/cli_new/lib/cli/plugins/agent/main.py > 59280ece8ebd00bb96df3675b6356a26cc48a2c0 > src/python/cli_new/lib/cli/plugins/task/main.py > cc6cff56c71262729a8870017bef2e97636abe5a > src/python/cli_new/lib/cli/tests/base.py > 89360e6cac5ca910044a5a82fab7237510edee7f > src/python/cli_new/lib/cli/tests/tests.py > 79e1036f6d11c63884091fe43672607b03955c1a > src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 > src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 > src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 > src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db > support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 > support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 > > > Diff: https://reviews.apache.org/r/67488/diff/12/ > > > Testing > --- > > Testing done on Fedora 25 with `python` being Python 2.7, `python3` being > Python 3.5 and `python36` being Python 3.6. > > > For Autotools: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java > $ make check > ``` > > For CMake: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36 > $ cmake --build . -- -j16 > $ ./src/mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > $ cmake --build . --target tests -- -j16 > $ ctest -R CLI > Test project /home/agrillet/apache-mesos/build > Start 4: CLITests > 1/1 Test #4: CLITests . Passed3.63 sec > > 100% tests passed, 0 tests failed out of 1 > ``` > > Checked that the the CLI tests were run, that the content of the directory > build/src/cli was as expected, and that build/src/mesos was correctly running. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67488: Updated CLI to Python 3.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67488/#review206105 --- Ship it! Ship It! - Kevin Klues On July 6, 2018, 1:56 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67488/ > --- > > (Updated July 6, 2018, 1:56 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > The build tools are also up to date thus the CLI can still be built > using Autotools and CMake. No features have been added to the CLI. > > > Diffs > - > > cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 > configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b > src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada > src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca > src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 > src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c > src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f > src/python/cli_new/lib/cli/config.py > 6f92622725d8a042a2a728fd38c977ac690ef6be > src/python/cli_new/lib/cli/docopt.py > 86a4e9c74326fb80cc59487113f07358dd96960d > src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b > src/python/cli_new/lib/cli/plugins/agent/main.py > 59280ece8ebd00bb96df3675b6356a26cc48a2c0 > src/python/cli_new/lib/cli/plugins/task/main.py > cc6cff56c71262729a8870017bef2e97636abe5a > src/python/cli_new/lib/cli/tests/base.py > 89360e6cac5ca910044a5a82fab7237510edee7f > src/python/cli_new/lib/cli/tests/tests.py > 79e1036f6d11c63884091fe43672607b03955c1a > src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 > src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 > src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 > src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db > support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 > support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 > > > Diff: https://reviews.apache.org/r/67488/diff/12/ > > > Testing > --- > > Testing done on Fedora 25 with `python` being Python 2.7, `python3` being > Python 3.5 and `python36` being Python 3.6. > > > For Autotools: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java > $ make check > ``` > > For CMake: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36 > $ cmake --build . -- -j16 > $ ./src/mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > $ cmake --build . --target tests -- -j16 > $ ctest -R CLI > Test project /home/agrillet/apache-mesos/build > Start 4: CLITests > 1/1 Test #4: CLITests . Passed3.63 sec > > 100% tests passed, 0 tests failed out of 1 > ``` > > Checked that the the CLI tests were run, that the content of the directory > build/src/cli was as expected, and that build/src/mesos was correctly running. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67906: Updated tox usage in mesos-style.py to run in support virtualenv.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67906/#review206073 --- Ship it! I've changed some small stylistic things in the commit before pushing it. Please see the commit logs for details. - Kevin Klues On July 13, 2018, 8:20 a.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67906/ > --- > > (Updated July 13, 2018, 8:20 a.m.) > > > Review request for mesos, Benjamin Bannier, Eric Chung, and Kevin Klues. > > > Bugs: MESOS-9073 > https://issues.apache.org/jira/browse/MESOS-9073 > > > Repository: mesos > > > Description > --- > > Updated tox usage in mesos-style.py to run in support virtualenv. > > > Diffs > - > > support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 > support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 > > > Diff: https://reviews.apache.org/r/67906/diff/3/ > > > Testing > --- > > Used `mesos-style` to check that tox was correctly running. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67910: Fixed virtualenv management in support directory.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67910/#review206072 --- Ship it! I've split this into 3 commits on my end and slightly changed some variable names in the commit that actually fixes the virtualenv management. Please see the commit logs for details. - Kevin Klues On July 13, 2018, 4:38 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67910/ > --- > > (Updated July 13, 2018, 4:38 p.m.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9075 > https://issues.apache.org/jira/browse/MESOS-9075 > > > Repository: mesos > > > Description > --- > > The switch from Python 2 to Python 3 creates problems regarding the > virtual environment if a developer dangles between Python 2 and 3 as > we want the virtual environment to use the same version of Python as > the user to give useful logs when linting. > > This commit fixes the issue by adding a new check in the function of > `mesos-style.py` called `should_build_virtualenv` to see if the Python > interpreter version currently used is the one in the virtual > environement. If not, the virtual environment will get recreated. > > > Diffs > - > > support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 > support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 > > > Diff: https://reviews.apache.org/r/67910/diff/2/ > > > Testing > --- > > Run the two `mesos-style.py` and `build-virtualenv` with > `MESOS_SUPPORT_PYTHON` set to 3 or not. Checked that the python intepreter > installed in the virtual environment was correct. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67911: Fixed `build-virtualenv` to correctly clear virtual environment.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67911/#review206071 --- Ship it! Ship It! - Kevin Klues On July 13, 2018, 4:37 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67911/ > --- > > (Updated July 13, 2018, 4:37 p.m.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-9075 > https://issues.apache.org/jira/browse/MESOS-9075 > > > Repository: mesos > > > Description > --- > > Running `build-virtualenv` with Python 3 then again with Python 2 > lets the directory with two versions of Python in the virtualenv > and Python 3 is then still used in that environment. > > This is due to an old issue of `virtualenv` that we now handle by > removing the support virtualenv directory before building it again. > > > Diffs > - > > support/build-virtualenv b8dc1d98f97bc70d9f7099db2c9cb85a884be12c > > > Diff: https://reviews.apache.org/r/67911/diff/1/ > > > Testing > --- > > Run `PYTHON=python3 build-virtualenv` then `build-virtualenv` then checked > `.virtualenv` content. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67853: Updated build-virtualenv to use Python 3 features.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67853/#review205856 --- Ship it! Ship It! - Kevin Klues On July 9, 2018, 3:54 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67853/ > --- > > (Updated July 9, 2018, 3:54 p.m.) > > > Review request for mesos, Benjamin Bannier, Kevin Klues, and Till Toenshoff. > > > Bugs: MESOS-9058 > https://issues.apache.org/jira/browse/MESOS-9058 > > > Repository: mesos > > > Description > --- > > With Python 3, virtualenv is built-in. We thus remove the error > messages if the script is run using Python 3 without virtualenv > installed on the machine. We also use `python` instead of > `virtualenv` to build the virtual environment. > > This change also fixes an issue on my computer where running > `build-virtualenv` with Python 3 displayed an error message: > `ModuleNotFoundError: No module named 'nodeenv'`. > > > Diffs > - > > support/build-virtualenv 5fda08187bb92847dba459550313edff756c9e7f > > > Diff: https://reviews.apache.org/r/67853/diff/4/ > > > Testing > --- > > Ran `build-virtualenv` with python 2 and 3. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67853: Updated build-virtualenv to use Python 3 features.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67853/#review205855
---
support/build-virtualenv
Line 52 (original), 41 (patched)
<https://reviews.apache.org/r/67853/#comment288759>
Can we replace `` with `${0}` both here and below?
- Kevin Klues
On July 9, 2018, 3:54 p.m., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67853/
> ---
>
> (Updated July 9, 2018, 3:54 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Kevin Klues, and Till Toenshoff.
>
>
> Bugs: MESOS-8882
> https://issues.apache.org/jira/browse/MESOS-8882
>
>
> Repository: mesos
>
>
> Description
> ---
>
> With Python 3, virtualenv is built-in. We thus remove the error
> messages if the script is run using Python 3 without virtualenv
> installed on the machine. We also use `python` instead of
> `virtualenv` to build the virtual environment.
>
> This change also fixes an issue on my computer where running
> `build-virtualenv` with Python 3 displayed an error message:
> `ModuleNotFoundError: No module named 'nodeenv'`.
>
>
> Diffs
> -
>
> support/build-virtualenv 5fda08187bb92847dba459550313edff756c9e7f
>
>
> Diff: https://reviews.apache.org/r/67853/diff/2/
>
>
> Testing
> ---
>
> Ran `build-virtualenv` with python 2 and 3.
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 67853: Updated build-virtualenv to use Python 3 features.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67853/#review205854 --- support/build-virtualenv Line 49 (original), 38 (patched) <https://reviews.apache.org/r/67853/#comment288758> I don't think we need this check in here. We should make this script usable for *both* python2 and python 3 environments and depending on what the environment is that it is invoked in, it will build an appropriate virtualenv. - Kevin Klues On July 9, 2018, 10:33 a.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67853/ > --- > > (Updated July 9, 2018, 10:33 a.m.) > > > Review request for mesos, Benjamin Bannier, Kevin Klues, and Till Toenshoff. > > > Bugs: MESOS-8882 > https://issues.apache.org/jira/browse/MESOS-8882 > > > Repository: mesos > > > Description > --- > > With Python 3, virtualenv is built-in. We thus remove the error > messages if the script is run using Python 3 without virtualenv > installed on the machine. We also use `python` instead of > `virtualenv` to build the virtual environment. > > This change also fixes an issue on my computer where running > `build-virtualenv` with Python 3 displayed an error message: > `ModuleNotFoundError: No module named 'nodeenv'`. > > > Diffs > - > > support/build-virtualenv 5fda08187bb92847dba459550313edff756c9e7f > > > Diff: https://reviews.apache.org/r/67853/diff/1/ > > > Testing > --- > > Ran `build-virtualenv` with python 2 and 3. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67842: Refactored base of Python CLI tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67842/#review205803 --- Ship it! I will fix the issues raised above myself before committing. - Kevin Klues On July 6, 2018, 1:53 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67842/ > --- > > (Updated July 6, 2018, 1:53 p.m.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > Refactored base of Python CLI tests. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 4ffa27ca95ddf6575fb0a844d6996890bed4d8c9 > > > Diff: https://reviews.apache.org/r/67842/diff/2/ > > > Testing > --- > > In `src/python/cli_new/`: > ``` > $ source activate > $ mesos-cli-tests > ``` > Removed one of the binary and tried again to check that the error was correct. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67842: Refactored base of Python CLI tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67842/#review205802 --- src/python/cli_new/lib/cli/tests/base.py Lines 200-202 (patched) <https://reviews.apache.org/r/67842/#comment288714> This should be in `Master.__del__()`, not `Master.kill()` src/python/cli_new/lib/cli/tests/base.py Lines 240-241 (original), 247-248 (patched) <https://reviews.apache.org/r/67842/#comment288713> These need to be wrapped in e.g.: ``` if hasattr(self, "flags") and hasattr(self.flags, "work_dir"): ``` - Kevin Klues On July 6, 2018, 1:53 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67842/ > --- > > (Updated July 6, 2018, 1:53 p.m.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > Refactored base of Python CLI tests. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 4ffa27ca95ddf6575fb0a844d6996890bed4d8c9 > > > Diff: https://reviews.apache.org/r/67842/diff/2/ > > > Testing > --- > > In `src/python/cli_new/`: > ``` > $ source activate > $ mesos-cli-tests > ``` > Removed one of the binary and tried again to check that the error was correct. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67842: Refactored base of Python CLI tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67842/#review205799 --- src/python/cli_new/lib/cli/tests/base.py Lines 282 (patched) <https://reviews.apache.org/r/67842/#comment288710> Can we move this below the try below? We should only down the refcount after we are sure the agent is gone. src/python/cli_new/lib/cli/tests/base.py Lines 397 (patched) <https://reviews.apache.org/r/67842/#comment288711> Same cooment as for the agent. - Kevin Klues On July 6, 2018, 10:02 a.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67842/ > --- > > (Updated July 6, 2018, 10:02 a.m.) > > > Review request for mesos and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > Refactored base of Python CLI tests. > > > Diffs > - > > src/python/cli_new/lib/cli/tests/base.py > 4ffa27ca95ddf6575fb0a844d6996890bed4d8c9 > > > Diff: https://reviews.apache.org/r/67842/diff/1/ > > > Testing > --- > > In `src/python/cli_new/`: > ``` > $ source activate > $ mesos-cli-tests > ``` > Removed one of the binary and tried again to check that the error was correct. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67488: Updated CLI to Python 3.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67488/#review205726 --- src/python/cli_new/lib/cli/tests/base.py Line 119 (original), 119 (patched) <https://reviews.apache.org/r/67488/#comment288605> Why did we change this? src/python/cli_new/lib/cli/tests/base.py Lines 143 (patched) <https://reviews.apache.org/r/67488/#comment288606> Why terminate instead of kill? Terminate shuts down gracefully, while kill forces the process to die immediately. - Kevin Klues On June 12, 2018, 4:29 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67488/ > --- > > (Updated June 12, 2018, 4:29 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > The build tools are also up to date thus the CLI can still be built > using Autotools and CMake. No features have been added to the CLI. > > The PyInstaller dependency has been updated due to issues with > PyInstaller 3.1.1 and Python 3.6. > > > Diffs > - > > cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 > configure.ac 8b8064aca7ae39e16dda40828b5a087b14b54a65 > src/Makefile.am de808205f59a485d758d1baf9d990bc631eb8c5c > src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca > src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 > src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c > src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f > src/python/cli_new/lib/cli/config.py > 6f92622725d8a042a2a728fd38c977ac690ef6be > src/python/cli_new/lib/cli/docopt.py > 86a4e9c74326fb80cc59487113f07358dd96960d > src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b > src/python/cli_new/lib/cli/plugins/agent/main.py > 59280ece8ebd00bb96df3675b6356a26cc48a2c0 > src/python/cli_new/lib/cli/plugins/base.py > e01a7b2bc4d4cbabe706c8926913f43d2b4cf69c > src/python/cli_new/lib/cli/plugins/task/main.py > cc6cff56c71262729a8870017bef2e97636abe5a > src/python/cli_new/lib/cli/tests/base.py > 4ffa27ca95ddf6575fb0a844d6996890bed4d8c9 > src/python/cli_new/lib/cli/tests/tests.py > 79e1036f6d11c63884091fe43672607b03955c1a > src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 > src/python/cli_new/pip-requirements.txt > aeb023325e838aa42f8d7418bb7f8293c3fa5614 > src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 > src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 > > > Diff: https://reviews.apache.org/r/67488/diff/7/ > > > Testing > --- > > Testing done on Fedora 25 with `python` being Python 2.7, `python3` being > Python 3.5 and `python36` being Python 3.6. > > > For Autotools: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java > $ make check > ``` > > For CMake: > > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36 > $ cmake --build . -- -j16 > $ ./src/mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > $ cmake --build . --target tests -- -j16 > $ ctest -R CLI > Test project /home/agrillet/apache-mesos/build > Start 4: CLITests > 1/1 Test #4: CLITests . Passed3.63 sec > > 100% tests passed, 0 tests failed out of 1 > ``` > > Checked that the the CLI tests were run, that the content of the directory > build/src/cli was as expected, and that build/src/mesos was correctly running. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67487: Used `$PYTHON` in configure.ac and Makefile.am.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67487/#review205724 --- Ship it! Ship It! - Kevin Klues On June 12, 2018, 2:51 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67487/ > --- > > (Updated June 12, 2018, 2:51 p.m.) > > > Review request for mesos, Eric Chung and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > This ensures that we use Python 2 even if the Python > intepreter under the command `python` is Python 3. > > > Diffs > - > > configure.ac 8b8064aca7ae39e16dda40828b5a087b14b54a65 > src/Makefile.am de808205f59a485d758d1baf9d990bc631eb8c5c > > > Diff: https://reviews.apache.org/r/67487/diff/7/ > > > Testing > --- > > In `build`: > ``` > $ ../configure --disable-java --enable-new-cli > $ GTEST_FILTER="" nice make -j16 check > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 67413: Refactored logic for `PYTHON` and `PYTHON_VERSION` in `configure.ac`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67413/#review205723 --- Ship it! Ship It! - Kevin Klues On July 3, 2018, 2:40 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67413/ > --- > > (Updated July 3, 2018, 2:40 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > This will facilitate the introduction of `PYTHON_3` and > `PYTHON_3_VERSION` to build the CLI in a future commit. > > > Diffs > - > > configure.ac 8b8064aca7ae39e16dda40828b5a087b14b54a65 > > > Diff: https://reviews.apache.org/r/67413/diff/10/ > > > Testing > --- > > My machine runs Python 2 when using `python` but `python3` is available. The > result of `configure` was: > > `../configure` works. > `PYTHON_VERSION=3 ../configure` returns `configure: error: Mesos requires > Python < 3.0`. > `PYTHON=python3 ../configure` returns `configure: error: Mesos requires > Python < 3.0`. > `PYTHON=python3 ../configure --disable-python` works. > `PYTHON=python3 ../configure --disable-python --enable-new-cli` returns > `configure: error: Mesos requires Python < 3.0`. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67413: Refactored logic for `PYTHON` and `PYTHON_VERSION` in `configure.ac`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67413/#review205722 --- configure.ac Lines 766 (patched) <https://reviews.apache.org/r/67413/#comment288600> Please remove this and move it to the next RR. configure.ac Line 2318 (original), 2320 (patched) <https://reviews.apache.org/r/67413/#comment288601> s/both went/both when/ configure.ac Line 2319 (original), 2321 (patched) <https://reviews.apache.org/r/67413/#comment288602> s/or/and when/ - Kevin Klues On July 3, 2018, 2:40 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67413/ > --- > > (Updated July 3, 2018, 2:40 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > This will facilitate the introduction of `PYTHON_3` and > `PYTHON_3_VERSION` to build the CLI in a future commit. > > > Diffs > - > > configure.ac 8b8064aca7ae39e16dda40828b5a087b14b54a65 > > > Diff: https://reviews.apache.org/r/67413/diff/9/ > > > Testing > --- > > My machine runs Python 2 when using `python` but `python3` is available. The > result of `configure` was: > > `../configure` works. > `PYTHON_VERSION=3 ../configure` returns `configure: error: Mesos requires > Python < 3.0`. > `PYTHON=python3 ../configure` returns `configure: error: Mesos requires > Python < 3.0`. > `PYTHON=python3 ../configure --disable-python` works. > `PYTHON=python3 ../configure --disable-python --enable-new-cli` returns > `configure: error: Mesos requires Python < 3.0`. > > > Thanks, > > Armand Grillet > >
Re: Review Request 67412: Improved coverage with configure and `PYTHON` or `PYTHON_VERSION` set.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67412/#review204843 --- Ship it! Ship It! - Kevin Klues On June 1, 2018, 3:14 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67412/ > --- > > (Updated June 1, 2018, 3:14 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > We set `PYTHON` and `PYTHON_VERSION` when configuring the build. > We now cover all possible cases (both variables set, only one, none). > This ensures that both variables are set after being checked. > > > Diffs > - > > configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123 > > > Diff: https://reviews.apache.org/r/67412/diff/3/ > > > Testing > --- > > I added two lines after the new code: > ``` > AC_MSG_NOTICE([$PYTHON]) > AC_MSG_NOTICE([$PYTHON_VERSION]) > ``` > > Then, when using `configure`, I've checked the output. > ``` > $ ../configure > ... > configure: /usr/bin/python > configure: 2.7 > $ PYTHON_VERSION=4 ../configure > ... > configure: python4 > configure: 4 > $ PYTHON=yolo ../configure > ... > configure: yolo > configure: > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 67412: Improved coverage with configure and `PYTHON` or `PYTHON_VERSION` set.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67412/#review204842 --- Fix it, then Ship it! Ship It! configure.ac Lines 2276-2277 (patched) <https://reviews.apache.org/r/67412/#comment287583> Changing this to the followign before I commit it: ``` +AC_CHECK_PROG([PYTHON_CHECK], [$PYTHON], [yes]) +AS_IF([test "x$PYTHON_CHECK" = "xyes"], [ + PYTHON_VERSION=`$PYTHON -c "import sys; sys.stdout.write(sys.version[[:3]])"` +], [ + AC_MSG_ERROR([Cannot find Python executable '$PYTHON' in path.]) +]) ``` - Kevin Klues On June 1, 2018, 3:14 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67412/ > --- > > (Updated June 1, 2018, 3:14 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > We set `PYTHON` and `PYTHON_VERSION` when configuring the build. > We now cover all possible cases (both variables set, only one, none). > This ensures that both variables are set after being checked. > > > Diffs > - > > configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123 > > > Diff: https://reviews.apache.org/r/67412/diff/3/ > > > Testing > --- > > I added two lines after the new code: > ``` > AC_MSG_NOTICE([$PYTHON]) > AC_MSG_NOTICE([$PYTHON_VERSION]) > ``` > > Then, when using `configure`, I've checked the output. > ``` > $ ../configure > ... > configure: /usr/bin/python > configure: 2.7 > $ PYTHON_VERSION=4 ../configure > ... > configure: python4 > configure: 4 > $ PYTHON=yolo ../configure > ... > configure: yolo > configure: > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 67411: Broadened check for Autotools Python environment variables.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67411/#review204837 --- Ship it! Ship It! - Kevin Klues On June 15, 2018, 1:16 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67411/ > --- > > (Updated June 15, 2018, 1:16 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > The checks now also apply if we run configure with disabled Python > bindings but enabled new CLI. Another check regarding the Python > version has been kept separated as we will use different Python > versions for both components in a later commit. > > > Diffs > - > > configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123 > > > Diff: https://reviews.apache.org/r/67411/diff/3/ > > > Testing > --- > > ``` > $ PYTHON=yolo PYTHON_VERSION=3 ../configure --disable-java --disable-python > --enable-new-cli > ... > configure: error: only specify one of PYTHON or PYTHON_VERSION > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 67487: Used `$PYTHON` in configure.ac and Makefile.am.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67487/#review204690 --- src/Makefile.am Lines 1804-1816 (original), 1804-1816 (patched) <https://reviews.apache.org/r/67487/#comment287302> Can you set PYTHON here so we know which python version we are building the virtualenv with? - Kevin Klues On June 12, 2018, 2:51 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67487/ > --- > > (Updated June 12, 2018, 2:51 p.m.) > > > Review request for mesos, Eric Chung and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > This ensures that we use Python 2 even if the Python > intepreter under the command `python` is Python 3. > > > Diffs > - > > configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123 > src/Makefile.am 2bcee1e0e198e6be009174570cdaa1c8b8372a71 > > > Diff: https://reviews.apache.org/r/67487/diff/3/ > > > Testing > --- > > In `build`: > ``` > $ ../configure --disable-java --enable-new-cli > $ GTEST_FILTER="" nice make -j16 check > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 67412: Improved coverage with configure and `PYTHON` or `PYTHON_VERSION` set.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67412/#review204688
---
configure.ac
Lines 2278 (patched)
<https://reviews.apache.org/r/67412/#comment287300>
Instead of this long complicated line, can we just set PYTHON_VERSION
directrly from:
```
PYTHON_VERSION=`$PYTHON -c "import sys;
sys.stdout.write(sys.version[[:3]])"`
```
- Kevin Klues
On June 1, 2018, 3:14 p.m., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67412/
> ---
>
> (Updated June 1, 2018, 3:14 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung,
> and Kevin Klues.
>
>
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
>
>
> Repository: mesos
>
>
> Description
> ---
>
> We set `PYTHON` and `PYTHON_VERSION` when configuring the build.
> We now cover all possible cases (both variables set, only one, none).
> This ensures that both variables are set after being checked.
>
>
> Diffs
> -
>
> configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123
>
>
> Diff: https://reviews.apache.org/r/67412/diff/2/
>
>
> Testing
> ---
>
> I added two lines after the new code:
> ```
> AC_MSG_NOTICE([$PYTHON])
> AC_MSG_NOTICE([$PYTHON_VERSION])
> ```
>
> Then, when using `configure`, I've checked the output.
> ```
> $ ../configure
> ...
> configure: /usr/bin/python
> configure: 2.7
> $ PYTHON_VERSION=4 ../configure
> ...
> configure: python4
> configure: 4
> $ PYTHON=yolo ../configure
> ...
> configure: yolo
> configure:
> ```
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 67411: Broadened check for Autotools Python environment variables.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67411/#review204686 --- Ship it! Ship It! - Kevin Klues On June 1, 2018, 3:06 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67411/ > --- > > (Updated June 1, 2018, 3:06 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, > and Kevin Klues. > > > Bugs: MESOS-8955 > https://issues.apache.org/jira/browse/MESOS-8955 > > > Repository: mesos > > > Description > --- > > The checks now also apply if we run configure with disabled Python > bindings but enabled new CLI. > > > Diffs > - > > configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123 > > > Diff: https://reviews.apache.org/r/67411/diff/2/ > > > Testing > --- > > ``` > $ PYTHON=yolo PYTHON_VERSION=3 ../configure --disable-java --disable-python > --enable-new-cli > ... > configure: error: only specify one of PYTHON or PYTHON_VERSION > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 67488: Updated CLI to Python 3.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67488/#review204685
---
src/python/cli_new/bootstrap
Lines 48-62 (original), 46-72 (patched)
<https://reviews.apache.org/r/67488/#comment287293>
Three is a better method of creating a virtualenv with python 3. Please see
https://github.com/dcos/dcos-cli/blob/992d516cec5b7e21154190ae121c3c85b73baa17/python/bin/env.sh#L36
for reference.
src/python/cli_new/lib/cli/tests/base.py
Lines 184-188 (original), 187-190 (patched)
<https://reviews.apache.org/r/67488/#comment287295>
Reverse the order of these.
src/python/cli_new/lib/cli/tests/base.py
Line 275 (original), 268-274 (patched)
<https://reviews.apache.org/r/67488/#comment287296>
Reverse this.
src/python/cli_new/lib/cli/tests/base.py
Lines 268-273 (patched)
<https://reviews.apache.org/r/67488/#comment287297>
Format similar to for Master with just 4 lines.
src/python/cli_new/lib/cli/tests/base.py
Lines 277-278 (original), 276-278 (patched)
<https://reviews.apache.org/r/67488/#comment287298>
Can you move this inside the try and have a finally that subtracts the
Agent count? This may not work, because we return from the try, but it's worth
a (sic) try.
src/python/cli_new/lib/cli/util.py
Lines 164-166 (original), 164-166 (patched)
<https://reviews.apache.org/r/67488/#comment287294>
Remove this comment
- Kevin Klues
On June 12, 2018, 4:29 p.m., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
>
> (Updated June 12, 2018, 4:29 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung,
> and Kevin Klues.
>
>
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
>
>
> Repository: mesos
>
>
> Description
> ---
>
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
>
> The PyInstaller dependency has been updated due to issues with
> PyInstaller 3.1.1 and Python 3.6.
>
>
> Diffs
> -
>
> cmake/CompilationConfigure.cmake 2f92acb3a140faa48e3639c7000be7f43020ad7d
> configure.ac f5a9d5bded40d2af6df7fe872395b076cbd37123
> src/Makefile.am 2bcee1e0e198e6be009174570cdaa1c8b8372a71
> src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca
> src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053
> src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c
> src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f
> src/python/cli_new/lib/cli/config.py
> 6f92622725d8a042a2a728fd38c977ac690ef6be
> src/python/cli_new/lib/cli/docopt.py
> 86a4e9c74326fb80cc59487113f07358dd96960d
> src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b
> src/python/cli_new/lib/cli/plugins/agent/main.py
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0
> src/python/cli_new/lib/cli/plugins/base.py
> e01a7b2bc4d4cbabe706c8926913f43d2b4cf69c
> src/python/cli_new/lib/cli/plugins/task/main.py
> cc6cff56c71262729a8870017bef2e97636abe5a
> src/python/cli_new/lib/cli/tests/base.py
> 4ffa27ca95ddf6575fb0a844d6996890bed4d8c9
> src/python/cli_new/lib/cli/tests/tests.py
> 79e1036f6d11c63884091fe43672607b03955c1a
> src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8
> src/python/cli_new/pip-requirements.txt
> aeb023325e838aa42f8d7418bb7f8293c3fa5614
> src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1
> src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91
>
>
> Diff: https://reviews.apache.org/r/67488/diff/2/
>
>
> Testing
> ---
>
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being
> Python 3.5 and `python36` being Python 3.6.
>
>
> For Autotools:
>
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
>
> For CMake:
>
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
>
> Usage:
> mesos (-h | --help)
> mesos --version
> mesos [...]
>
> Options:
> -h --help Show this screen.
> --version Show version info.
>
> Commands:
> agent
Re: Review Request 67054: Added .tox to files excluded by Python linter.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67054/#review202925 --- This looks good, but I think we should also add the .virtualenv and .tox directories to our .gitignore. They should never be part of the list of files that are being processed by mesos-style.py during a commit hook anyway. - Kevin Klues On May 10, 2018, 10:56 a.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67054/ > --- > > (Updated May 10, 2018, 10:56 a.m.) > > > Review request for mesos, Eric Chung and Kevin Klues. > > > Repository: mesos > > > Description > --- > > Added .tox to files excluded by Python linter. > > > Diffs > - > > support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d > > > Diff: https://reviews.apache.org/r/67054/diff/1/ > > > Testing > --- > > Before change, when running `python mesos-style.py`: > ``` > Checking 1769 Python files > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pylint/test/functional/redefined_builtin.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: """Tests for redefining builtins.""" > from __future__ import print_function > > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/astroid/tests/testdata/python2/data/all.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: > name = 'a' > _bla = 2 > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/_pytest/monkeypatch.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: """ monkeypatching and mocking functionality. > """ > from __future__ import absolute_import, division, print_function > > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/PyInstaller/lib/altgraph/Graph.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: """ > altgraph.Graph - Base Graph class > = > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/_vendor/requests/packages/chardet/euctwprober.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: BEGIN LICENSE BLOCK > > # The Original Code is mozilla.org code. > # > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/astroid/tests/testdata/python2/data/package/absimport.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: from __future__ import absolute_import, > print_function > import import_package_subpackage_module # fail > print(import_package_subpackage_module) > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/kazoo/protocol/states.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: """Kazoo State and Event objects""" > from collections import namedtuple > > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pylint/test/input/func_no_dummy_redefined.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: """Make sure warnings about redefinitions do not > trigger for dummy variables.""" > from __future__ import print_function > > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/_vendor/requests/hooks.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: # -*- coding: utf-8 -*- > > """ > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/_vendor/lockfile/__init__.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: # -*- coding: utf-8 -*- > > """ > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pygments/lexers/installers.py:1: > A license header should appear's on one of the first line of the file > starting with '# Licensed'.: # -*- coding: utf-8 -*- > """ > pygments.lexers.installers > src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/wheel/wininst2w
Re: Review Request 67032: Removed pip install requirements-test.in in new CLI bootstrap script.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67032/#review202753 --- Ship it! Ship It! - Kevin Klues On May 9, 2018, 1:13 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67032/ > --- > > (Updated May 9, 2018, 1:13 p.m.) > > > Review request for mesos and Kevin Klues. > > > Repository: mesos > > > Description > --- > > Removed pip install requirements-test.in in new CLI bootstrap script. > > > Diffs > - > > src/python/cli_new/bootstrap 87ac3cae790144fe642c9eb86106b05d60a9d14d > > > Diff: https://reviews.apache.org/r/67032/diff/1/ > > > Testing > --- > > Building the new CLI e.g., with cmake from a fresh build dir fails without > this patch, but works with it. > > > Thanks, > > Armand Grillet > >
Re: Review Request 64970: Use tox for linting and testing code living uder src/python.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64970/#review202749 --- support/mesos-style.py Lines 379 (patched) <https://reviews.apache.org/r/64970/#comment284733> I am about to commit this, but I wanted to note some changes for posterity. I am adding a bump to tox 3.0.0 as a precommit to this RR so that we can additionally pass the '-qq' to tox to suppress its output. I am also modifying this RR to pass this option here. We want to see the output of the commands it issues, but we don't want to see the output of tox itself. The '-qq' option gives us this ability. - Kevin Klues On March 14, 2018, 3:05 p.m., Eric Chung wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64970/ > --- > > (Updated March 14, 2018, 3:05 p.m.) > > > Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues. > > > Repository: mesos > > > Description > --- > > Use tox for linting and testing code living uder src/python. > > At the moment, all linting is done through the same `pylint` > installation under support/.virtualenv, which requires ALL dependencies > (i.e. pip-requirements.txt, requirements.in scattered in various > directories) to be installed in the same virtualenv, making things > really messy -- e.g. when I've changed some code under `src/python/lib`, > but don't have the dev virtualenv activated, linting will fail since > none of the dependencies under `src/python/lib` have been installed. > > Using tox, we can solve this problem by distributing a "test spec" > (tox.ini) in each of the python source directories which are aware of > its local dependencies only. To test or lint the code there would be as > simple as running `tox -e py27-lint `, and the corresponding > virtualenv and test dependencies would automatically be setup. > > This patch modifies `support/mesos-style.py` to install `tox` in > `support/.virtualenv` and delegates linting to a `tox` call when it sees > python directories that have tox setup for it. Linting for all other > languages will not be effected. > > Testing Done: > 1. intentionally create a lint error, such as extra spaces before a > parens in a python file > 2. run the pre-commit hook and see tox in action > > Reviewed at https://reviews.apache.org/r/64970/ > > > Diffs > - > > src/python/cli_new/tests/__init__.py PRE-CREATION > src/python/cli_new/tox.ini PRE-CREATION > src/python/lib/requirements-test.in > b2b73aab65377d9310797203ea84c5150ae60805 > src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db > support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f > > > Diff: https://reviews.apache.org/r/64970/diff/6/ > > > Testing > --- > > 1. intentionally create a lint error, such as extra spaces before a parens in > a python file > 2. run the pre-commit hook and see tox in action > > > Thanks, > > Eric Chung > >
Re: Review Request 66649: Added pb2gen.sh for generating python protobuf bindings.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66649/#review202748 --- src/python/lib/pb2gen.sh Lines 64 (patched) <https://reviews.apache.org/r/66649/#comment284731> Is there some way to simplify this. Maybe by breaking it out into multiple steps? It will be really hard to maintain this going forward otherwise. src/python/lib/pb2gen.sh Lines 76 (patched) <https://reviews.apache.org/r/66649/#comment284732> Is there some way to simplify this. Maybe by breaking it out into multiple steps? It will be really hard to maintain this going forward otherwise. - Kevin Klues On April 17, 2018, 8:22 p.m., Eric Chung wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66649/ > --- > > (Updated April 17, 2018, 8:22 p.m.) > > > Review request for mesos, Armand Grillet, Jason Lai, Kevin Klues, and Zhitao > Li. > > > Repository: mesos > > > Description > --- > > The current state of support for python in protoc has two serious > issues: > 1. The `__init__.py` files necessary to mark a directory as a python > package aren't generated. > 2. The import paths in each of the generated .py files do not reflect > the `--python_path` option passed to `protoc`. > > This results in incomplete code generated, preventing it from being used > out of the box. To address this issue, we're adding a `pb2gen.sh` script > to do end-to-end code generation for python protobuf bindings: the > script generates the bindings based on what's in the `include` dir, then > postprocesses the generated code to add proper import paths and the > `__init__.py` files. > > > Diffs > - > > src/python/lib/pb2gen.sh PRE-CREATION > src/python/lib/requirements.in 0742f3d846c99c1c4907d9628fb49845564563b2 > > > Diff: https://reviews.apache.org/r/66649/diff/3/ > > > Testing > --- > > 1. under `src/python/lib`, run `pb2gen.sh` > 2. initialize and activate virtualenv: `virtualenv env && . env/bin/activate` > 3. install reqs: `pip install -r requirements.in` > 4. try to import modules from generated python code: `python -c 'from > mesos.pb2.mesos.v1.master import master_pb2'` > > > Thanks, > > Eric Chung > >
Re: Review Request 64211: Added options to build the Python CLI and run unit tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64211/#review201000 --- Ship it! Ship It! - Kevin Klues On March 24, 2018, 11:39 a.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64211/ > --- > > (Updated March 24, 2018, 11:39 a.m.) > > > Review request for mesos, Benjamin Bannier and Kevin Klues. > > > Bugs: MESOS-8240 > https://issues.apache.org/jira/browse/MESOS-8240 > > > Repository: mesos > > > Description > --- > > An update of the discarded review /r/52543. > > Works with Autotools and CMake. > > > Diffs > - > > CMakeLists.txt 6702f02245e3867c06bbd9efbbf4e3b961a7d9aa > cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c > configure.ac f0f901f2e565352c2804cae7b2ac255da81ce45d > docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 > src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 > src/Makefile.am 257ff0e8d21e5240c27a123ed0cd563214e24fce > src/python/cli_new/CMakeLists.txt PRE-CREATION > src/python/cli_new/tests/CMakeLists.txt PRE-CREATION > > > Diff: https://reviews.apache.org/r/64211/diff/18/ > > > Testing > --- > > Testing done on Fedora 25. > > For Autotools: > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ ../configure --enable-new-cli --disable-java --disable-python > $ make check > ``` > > For CMake: > ``` > $ ./bootstrap > $ mkdir build > $ cd build > $ cmake .. -DENABLE_NEW_CLI=1 > $ cmake --build . -- -j16 > $ ./src/mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > $ cmake --build . --target tests -- -j16 > $ ctest -R CLI > Test project /home/agrillet/apache-mesos/build > Start 4: CLITests > 1/1 Test #4: CLITests . Passed3.63 sec > > 100% tests passed, 0 tests failed out of 1 > ``` > > Checked that the the CLI tests were run, that the content of the directory > `build/src/cli` was as expected, and that `build/src/mesos` was correctly > running. > > > Thanks, > > Armand Grillet > >
Re: Review Request 65585: Improved documentation regarding the new CLI setup.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65585/#review200999 --- src/python/cli_new/README.md Lines 40-55 (patched) <https://reviews.apache.org/r/65585/#comment281931> I am going to change this to "~/.mesos-cli-venv" before I commit it. src/python/cli_new/bin/mesos Line 5 (original) <https://reviews.apache.org/r/65585/#comment281932> This should probably have been in a spearate commit since it's a functionality change and not just documentation. src/python/cli_new/bin/mesos-cli-tests Line 5 (original) <https://reviews.apache.org/r/65585/#comment281933> This should probably have been in a spearate commit since it's a functionality change and not just documentation. - Kevin Klues On April 12, 2018, 12:36 p.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65585/ > --- > > (Updated April 12, 2018, 12:36 p.m.) > > > Review request for mesos, Benjamin Bannier and Kevin Klues. > > > Bugs: MESOS-8240 > https://issues.apache.org/jira/browse/MESOS-8240 > > > Repository: mesos > > > Description > --- > > Explains how to create the necessary virtual environment from > anywhere and how to set up autocompletion in such case. > > Also removes an unnecessary activation of the virtual environment > in `mesos` and `mesos-cli-tests`. > > > Diffs > - > > src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 > src/python/cli_new/bin/mesos c5152a2ebf8704c804bb4f39e46580a512aecdea > src/python/cli_new/bin/mesos-cli-tests > 07659e0b4551c2381828b256608d2c6ced3ae745 > src/python/cli_new/bootstrap 9329a61a2a1f05286b4ff6e5fe68cd86ed48859a > > > Diff: https://reviews.apache.org/r/65585/diff/6/ > > > Testing > --- > > On Fedora 25: > ``` > apache-mesos (MESOS-8240)$ cd src/python/cli_new/ > cli_new (MESOS-8240)$ ./bootstrap > cli_new (MESOS-8240)$ source .virtualenv/activate > (mesos-cli) cli_new (MESOS-8240)$ mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > (mesos-cli) cli_new (MESOS-8240)$ source deactivate > cli_new (MESOS-8240)$ rm -rf .virtualenv/ > cli_new (MESOS-8240)$ cd .. > python (MESOS-8240)$ VIRTUALENV_DIRECTORY=$(pwd)/.venv ./cli_new/bootstrap > python (MESOS-8240)$ source > /home/agrillet/apache-mesos/src/python/.venv/bin/activate > python (MESOS-8240)$ source > /home/agrillet/apache-mesos/src/python/.venv/bin/postactivate > (mesos-cli) python (MESOS-8240)$ mesos > Mesos CLI > > Usage: > mesos (-h | --help) > mesos --version > mesos [...] > > Options: > -h --help Show this screen. > --version Show version info. > > Commands: > agent Interacts with the Mesos agents > config Interacts with the Mesos CLI configuration file > taskInteracts with the tasks running in a Mesos cluster > > See 'mesos help ' for more information on a specific command. > ``` > > > Thanks, > > Armand Grillet > >
Re: Review Request 65585: Improved documentation regarding the new CLI setup.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65585/#review200997
---
src/python/cli_new/bootstrap
Lines 120 (patched)
<https://reviews.apache.org/r/65585/#comment281927>
This should be ${CURRDIR} too.
There is no need to introduce a new ${BINDIR} here.
- Kevin Klues
On March 24, 2018, 11:39 a.m., Armand Grillet wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65585/
> ---
>
> (Updated March 24, 2018, 11:39 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Kevin Klues.
>
>
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Explains how to create the necessary virtual environment from
> anywhere and how to set up autocompletion in such case.
>
> Also removes an unnecessary activation of the virtual environment
> in `mesos` and `mesos-cli-tests`.
>
>
> Diffs
> -
>
> src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61
> src/python/cli_new/bin/mesos c5152a2ebf8704c804bb4f39e46580a512aecdea
> src/python/cli_new/bin/mesos-cli-tests
> 07659e0b4551c2381828b256608d2c6ced3ae745
> src/python/cli_new/bootstrap 9329a61a2a1f05286b4ff6e5fe68cd86ed48859a
>
>
> Diff: https://reviews.apache.org/r/65585/diff/5/
>
>
> Testing
> ---
>
> On Fedora 25:
> ```
> apache-mesos (MESOS-8240)$ cd src/python/cli_new/
> cli_new (MESOS-8240)$ ./bootstrap
> cli_new (MESOS-8240)$ source .virtualenv/activate
> (mesos-cli) cli_new (MESOS-8240)$ mesos
> Mesos CLI
>
> Usage:
> mesos (-h | --help)
> mesos --version
> mesos [...]
>
> Options:
> -h --help Show this screen.
> --version Show version info.
>
> Commands:
> agent Interacts with the Mesos agents
> config Interacts with the Mesos CLI configuration file
> taskInteracts with the tasks running in a Mesos cluster
>
> See 'mesos help ' for more information on a specific command.
> (mesos-cli) cli_new (MESOS-8240)$ source deactivate
> cli_new (MESOS-8240)$ rm -rf .virtualenv/
> cli_new (MESOS-8240)$ cd ..
> python (MESOS-8240)$ VIRTUALENV_DIRECTORY=$(pwd)/.venv ./cli_new/bootstrap
> python (MESOS-8240)$ source
> /home/agrillet/apache-mesos/src/python/.venv/bin/activate
> python (MESOS-8240)$ source
> /home/agrillet/apache-mesos/src/python/.venv/bin/postactivate
> (mesos-cli) python (MESOS-8240)$ mesos
> Mesos CLI
>
> Usage:
> mesos (-h | --help)
> mesos --version
> mesos [...]
>
> Options:
> -h --help Show this screen.
> --version Show version info.
>
> Commands:
> agent Interacts with the Mesos agents
> config Interacts with the Mesos CLI configuration file
> taskInteracts with the tasks running in a Mesos cluster
>
> See 'mesos help ' for more information on a specific command.
> ```
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 65705: Fixed CLI bootstrap script to work with long workspace paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65705/#review200996 --- Ship it! Ship It! - Kevin Klues On March 24, 2018, 11:39 a.m., Armand Grillet wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65705/ > --- > > (Updated March 24, 2018, 11:39 a.m.) > > > Review request for mesos, Benjamin Bannier and Kevin Klues. > > > Bugs: MESOS-8240 > https://issues.apache.org/jira/browse/MESOS-8240 > > > Repository: mesos > > > Description > --- > > If the absolute path to the Python interpreter is long, it may exceed > the maximum length allowed for a shebang line (limit set to 128 on many > Linux distributions). When the shebang length limit is exceeded, pip > fails with the error: "bad interpreter: No such file or directory". > > A work-around for this problem is to run the pip library module which is > what we do in this patch. This will also be done to use PyInstaller. > > > Diffs > - > > src/python/cli_new/bootstrap 9329a61a2a1f05286b4ff6e5fe68cd86ed48859a > > > Diff: https://reviews.apache.org/r/65705/diff/4/ > > > Testing > --- > > Tested on internal CI. > > > Thanks, > > Armand Grillet > >
