Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-24 Thread Till Toenshoff 

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



src/tests/slave_tests.cpp (lines 2109 - 2110)


Test LGTM - glad you came up with this so quick.


- Till Toenshoff


On June 24, 2015, 10:01 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35567/
> ---
> 
> (Updated June 24, 2015, 10:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> c5ffc49899976225e319f122cb5f7556c3b09d46 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/containerizer.cpp 68c6f989ce2f786771ee2286c98077bd59f4db44 
>   src/tests/slave_tests.cpp 027da84e5776cb3e98cf23a179238fbe75417d1a 
> 
> Diff: https://reviews.apache.org/r/35567/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-24 Thread Benjamin Hindman 


> On June 23, 2015, 12:35 a.m., Till Toenshoff wrote:
> > src/tests/containerizer.cpp, lines 127-128
> > 
> >
> > Cant follow this one - are you saying that  `executorEnvironment(..)` 
> > should possibly set environment variables?

I enhanced the comment.


- Benjamin


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


On June 24, 2015, 10:01 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35567/
> ---
> 
> (Updated June 24, 2015, 10:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> c5ffc49899976225e319f122cb5f7556c3b09d46 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/containerizer.cpp 68c6f989ce2f786771ee2286c98077bd59f4db44 
>   src/tests/slave_tests.cpp 027da84e5776cb3e98cf23a179238fbe75417d1a 
> 
> Diff: https://reviews.apache.org/r/35567/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-24 Thread Benjamin Hindman 

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

(Updated June 24, 2015, 10:01 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Updates from review comments and added a test!


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
c5ffc49899976225e319f122cb5f7556c3b09d46 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/tests/containerizer.cpp 68c6f989ce2f786771ee2286c98077bd59f4db44 
  src/tests/slave_tests.cpp 027da84e5776cb3e98cf23a179238fbe75417d1a 

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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-22 Thread Till Toenshoff 

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

Ship it!



src/slave/flags.cpp (line 28)


IIUC, then this should be the first include.



src/tests/containerizer.cpp (lines 127 - 128)


Cant follow this one - are you saying that  `executorEnvironment(..)` 
should possibly set environment variables?


- Till Toenshoff


On June 20, 2015, 7 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35567/
> ---
> 
> (Updated June 20, 2015, 7 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> c5ffc49899976225e319f122cb5f7556c3b09d46 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 
> 
> Diff: https://reviews.apache.org/r/35567/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-20 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [35561, 35562, 35563, 35564]

Failed command: ./support/apply-review.sh -n -r 35564

Error:
 2015-06-20 22:05:25 URL:https://reviews.apache.org/r/35564/diff/raw/ 
[7555/7555] -> "35564.patch" [1]
error: patch failed: src/slave/containerizer/containerizer.cpp:276
error: src/slave/containerizer/containerizer.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On June 20, 2015, 7 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35567/
> ---
> 
> (Updated June 20, 2015, 7 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> c5ffc49899976225e319f122cb5f7556c3b09d46 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 
> 
> Diff: https://reviews.apache.org/r/35567/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

2015-06-20 Thread Benjamin Hindman 

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

(Updated June 20, 2015, 7 p.m.)


Review request for mesos and Till Toenshoff.


Summary (updated)
-

Added 'executor_environment_variables' flag to slave.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
c5ffc49899976225e319f122cb5f7556c3b09d46 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 

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


Testing
---

make check


Thanks,

Benjamin Hindman