Re: Review Request 43935: Allow setting role in mesos-execute.

2016-04-09 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review128009
---


Ship it!




Ship It!

- Michael Park


On April 9, 2016, 7:14 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated April 9, 2016, 7:14 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp df93e92252addaa794898ba95604eb2f91284b87 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-04-09 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review127979
---



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 9, 2016, 7:14 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated April 9, 2016, 7:14 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp df93e92252addaa794898ba95604eb2f91284b87 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-04-09 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 四月 9, 2016, 7:14 a.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Changes
---

rebase


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp df93e92252addaa794898ba95604eb2f91284b87 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-30 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review126202
---


Fix it, then Ship it!





src/cli/execute.cpp (line 483)


It looks like this patch needs to be rebased now, sorry about the delay, 
but I'll get this committed once it's rebased!


- Michael Park


On March 28, 2016, 1:11 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 28, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-28 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review125698
---



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 28, 2016, 1:11 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 28, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-28 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 三月 28, 2016, 1:11 p.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-27 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 三月 28, 2016, 1:30 a.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Changes
---

Fix michale's comment


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-26 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review125545
---




src/cli/execute.cpp (line 129)


Other example frameworks seem to use `"Role to use when registering"`. Can 
we keep it consistent?


- Michael Park


On March 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-25 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review125449
---



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-25 Thread Greg Mann 


> On March 24, 2016, 7:57 p.m., Greg Mann wrote:
> > src/cli/execute.cpp, line 215
> > 
> >
> > s/sepcified/specified/
> 
> Jian Qiu wrote:
> Sorry for the typo... please review again.

No worries :-) The patch looks good to me, perhaps mcypark can have a look when 
he has some cycles. You could contact him directly or ping him on the JIRA 
ticket to see when he'll have some time to review.


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review125329
---


On March 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-24 Thread Jian Qiu 


> On 三月 24, 2016, 7:57 p.m., Greg Mann wrote:
> > src/cli/execute.cpp, line 215
> > 
> >
> > s/sepcified/specified/

Sorry for the typo... please review again.


- Jian


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review125329
---


On 三月 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated 三月 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-24 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 三月 25, 2016, 1:54 a.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Changes
---

Fix a typo in the comment.


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-24 Thread Greg Mann 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review125329
---


Fix it, then Ship it!





src/cli/execute.cpp (line 214)


s/sepcified/specified/


- Greg Mann


On March 22, 2016, 2:57 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 22, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-22 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review124758
---



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 2:57 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 22, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-21 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 三月 22, 2016, 2:57 a.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Changes
---

address greg's comment


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-21 Thread Greg Mann 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review124613
---



Looks great! A couple small comments below.


src/cli/execute.cpp (lines 208 - 209)


Fits on one line.



src/cli/execute.cpp (lines 215 - 216)


Could you add a comment here to explain the behavior that you and Guangya 
discussed previously, i.e. the behavior of `Resources::find` with respect to 
the framework's role?


- Greg Mann


On March 17, 2016, 1:39 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 17, 2016, 1:39 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-19 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review124007
---



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 17, 2016, 1:39 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 17, 2016, 1:39 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-19 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 三月 17, 2016, 1:39 a.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing (updated)
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-15 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

(Updated 三月 15, 2016, 2:07 p.m.)


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-15 Thread Jian Qiu 


> On 三月 15, 2016, 12:26 p.m., Guangya Liu wrote:
> > src/cli/execute.cpp, lines 216-222
> > 
> >
> > Do you mean that here the framework task can only use reserved 
> > resources if the framework has a role? What about regular resources?
> > 
> > What about adding some logic as this:
> > 1) First use reserved resources for a role
> > 2) If no reserved resources, use regular resources?

resources.find() actually find the resources in the target role, then 
unreserved resources.


- Jian


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review123644
---


On 三月 15, 2016, 2:07 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated 三月 15, 2016, 2:07 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

2016-03-15 Thread Guangya Liu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/#review123644
---




src/cli/execute.cpp (line 129)


s/to use/to be used to keep consistent with above?



src/cli/execute.cpp (lines 215 - 221)


Do you mean that here the framework task can only use reserved resources if 
the framework has a role? What about regular resources?

What about adding some logic as this:
1) First use reserved resources for a role
2) If no reserved resources, use regular resources?


- Guangya Liu


On 三月 15, 2016, 11:55 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated 三月 15, 2016, 11:55 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Review Request 43935: Allow setting role in mesos-execute.

2016-03-15 Thread Jian Qiu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43935/
---

Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Bugs: MESOS-4744
https://issues.apache.org/jira/browse/MESOS-4744


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

Diff: https://reviews.apache.org/r/43935/diff/


Testing
---

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu