Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

2016-02-17 Thread Klaus Ma 

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

(Updated Feb. 18, 2016, 7:45 a.m.)


Review request for mesos, Ben Mahler and Joseph Wu.


Changes
---

rebase for code conflict


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca 
  src/slave/slave.cpp f0be0d5bf4b853952caf06e2e262c2903d79ead3 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

2016-01-18 Thread Klaus Ma 

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

(Updated Jan. 19, 2016, 10:55 a.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase for apply conflict


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp 98360b690382ed1912a868ac93b058cb28003a12 
  src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

2016-01-17 Thread Klaus Ma 

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

(Updated Jan. 18, 2016, 3:19 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Address comments


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

2016-01-15 Thread Klaus Ma 

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

(Updated Jan. 16, 2016, 1:38 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Add period to the summary.


Summary (updated)
-

MESOS-1718: use command line executor to launch tasks.


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

2016-01-14 Thread Klaus Ma 

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

(Updated Jan. 15, 2016, 1:18 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

2015-12-14 Thread Klaus Ma 


> On Dec. 14, 2015, 3:05 p.m., Qian Zhang wrote:
> > src/master/validation.cpp, line 331
> > 
> >
> > So now we allow a task has both ExecutorInfo and CommandInfo, right? If 
> > so, then you may need to update the following comments for TaskInfo as well.
> > /**
> >  * ... Either ExecutorInfo or CommandInfo should be set.
> >  * ...
> >  */
> > message TaskInfo {
> > ...
> > }
> > 
> > However, I still prefer the old way: either ExecutorInfo or CommandInfo 
> > should be set rather than both. So I think in `Master::_accept()` you can 
> > separate `validateExecutorInfo()` from `validation::task::validate()`, and 
> > call it separately before adding command executor into command task.

Yes, we used to add a new object into `ExecutorInfo::task_command` for task and 
keep this validation; but the validation seems not necessry after discussing 
with Vinode. Anyway, I think this's an option discussion for this JIRA.

@Vinode/@Joseph, any comments on this?


- Klaus


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


On Dec. 12, 2015, 5:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41306/
> ---
> 
> (Updated Dec. 12, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: use command line executor to launch tasks
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 
> 
> Diff: https://reviews.apache.org/r/41306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

2015-12-13 Thread Qian Zhang 

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



src/master/validation.cpp 


So now we allow a task has both ExecutorInfo and CommandInfo, right? If so, 
then you may need to update the following comments for TaskInfo as well.
/**
 * ... Either ExecutorInfo or CommandInfo should be set.
 * ...
 */
message TaskInfo {
...
}

However, I still prefer the old way: either ExecutorInfo or CommandInfo 
should be set rather than both. So I think in `Master::_accept()` you can 
separate `validateExecutorInfo()` from `validation::task::validate()`, and call 
it separately before adding command executor into command task.


- Qian Zhang


On Dec. 12, 2015, 5:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41306/
> ---
> 
> (Updated Dec. 12, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: use command line executor to launch tasks
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 
> 
> Diff: https://reviews.apache.org/r/41306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

2015-12-13 Thread Qian Zhang 

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



src/slave/slave.cpp 


So the method Containerizer::launch() with both taskInfo and executorInfo 
as parameters will never be called, and we only need to call the method 
Containerizer::launch() with only taskInfo as parameter, right?
But it seems not correct to me, e.g., in the code of Docker Containerizer 
(`DockerContainerizerProcess::launch()`), it actually needs taskinfo.


- Qian Zhang


On Dec. 12, 2015, 5:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41306/
> ---
> 
> (Updated Dec. 12, 2015, 5:55 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: use command line executor to launch tasks
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 
> 
> Diff: https://reviews.apache.org/r/41306/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>