Re: Review Request 46298: Rejected relative path agent work_dir.

2016-09-25 Thread Klaus Ma

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



Discard this PR for now. Will update it after `Flags.load()` refactor.

- Klaus Ma


On July 16, 2016, 9:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 16, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-16 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [46298]

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

- Mesos ReviewBot


On July 16, 2016, 1:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 16, 2016, 1:33 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-16 Thread Klaus Ma


> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
> Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?
> 
> Klaus Ma wrote:
> Found a `add` function to add required field; this function need an 
> additional "optional alias" to avoid conflict.
> 
> Jie Yu wrote:
> we can introduce another overload for `add`, similar to this one:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L182-L194
> 
> But, it accepts a validate function:
> ```
> template 
>   void add(
>   T1* t1,
>   const Name& name,
>   const std::string& help,
>   F validate)
>   {
> add(t1,
> name,
> None(),
> help,
> static_cast(nullptr),
> validate);
>   }
> ```
> 
> Klaus Ma wrote:
> @Jie, it will conflict with this one: 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L172-L180
>  . There's not enough information to distinguish `F validate` vs. `const T2& 
> t2`.
> 
> Jie Yu wrote:
> Aha, ic, now I get what you said about 'Option'. Yeah, can we make t2 an 
> optional field here:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L173

@Jie, try `Option` today, but c++ template can not distinguish `Option<...>` 
and validated lamba fuctions. Should we commit firstly or make it dependent on 
MESOS-5841.


- Klaus


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


On July 16, 2016, 9:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 16, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-16 Thread Klaus Ma

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

(Updated July 16, 2016, 9:33 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Rebase


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs (updated)
-

  src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 

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


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-13 Thread Jie Yu


> On July 11, 2016, 8:23 p.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
> Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?
> 
> Klaus Ma wrote:
> Found a `add` function to add required field; this function need an 
> additional "optional alias" to avoid conflict.
> 
> Jie Yu wrote:
> we can introduce another overload for `add`, similar to this one:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L182-L194
> 
> But, it accepts a validate function:
> ```
> template 
>   void add(
>   T1* t1,
>   const Name& name,
>   const std::string& help,
>   F validate)
>   {
> add(t1,
> name,
> None(),
> help,
> static_cast(nullptr),
> validate);
>   }
> ```
> 
> Klaus Ma wrote:
> @Jie, it will conflict with this one: 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L172-L180
>  . There's not enough information to distinguish `F validate` vs. `const T2& 
> t2`.

Aha, ic, now I get what you said about 'Option'. Yeah, can we make t2 an 
optional field here:
https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L173


- Jie


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


On July 13, 2016, 5:45 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 13, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-13 Thread Klaus Ma


> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
> Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?
> 
> Klaus Ma wrote:
> Found a `add` function to add required field; this function need an 
> additional "optional alias" to avoid conflict.
> 
> Jie Yu wrote:
> we can introduce another overload for `add`, similar to this one:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L182-L194
> 
> But, it accepts a validate function:
> ```
> template 
>   void add(
>   T1* t1,
>   const Name& name,
>   const std::string& help,
>   F validate)
>   {
> add(t1,
> name,
> None(),
> help,
> static_cast(nullptr),
> validate);
>   }
> ```

@Jie, it will conflict with this one: 
https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L172-L180
 . There's not enough information to distinguish `F validate` vs. `const T2& 
t2`.


- Klaus


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


On July 13, 2016, 1:45 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 13, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-13 Thread Jie Yu


> On July 11, 2016, 8:23 p.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
> Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?
> 
> Klaus Ma wrote:
> Found a `add` function to add required field; this function need an 
> additional "optional alias" to avoid conflict.

we can introduce another overload for `add`, similar to this one:
https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L182-L194

But, it accepts a validate function:
```
template 
  void add(
  T1* t1,
  const Name& name,
  const std::string& help,
  F validate)
  {
add(t1,
name,
None(),
help,
static_cast(nullptr),
validate);
  }
```


- Jie


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


On July 13, 2016, 5:45 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 13, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-12 Thread Klaus Ma


> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
> Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?

Found a `add` function to add required field; this function need an additional 
"optional alias" to avoid conflict.


- Klaus


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


On July 13, 2016, 1:45 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 13, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-12 Thread Klaus Ma

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

(Updated July 13, 2016, 1:45 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Using validate lamba to reject relative path.


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs (updated)
-

  src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 

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


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-12 Thread Jie Yu


> On July 11, 2016, 8:23 p.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?

Oh, I don't realize that we don't have validation support for required field 
without default value.

Can we introduce another `add` variant in flags that support the above case? Is 
that difficult?


- Jie


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


On April 19, 2016, 7:31 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 19, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-12 Thread Klaus Ma


> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > 
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.

@jie, thanks for the comments. Try to use validated lamba today, but there some 
issues:

1. The `work_dir` is required without default value; if using validate lamba, a 
"default" value is necessary: `add(T1* t1, name, alias, defaultValue, lamba)`
2. The default value is pass by const reference currently, we can not pass some 
empty value to it, e.g. nullptr or 0

For the default value, I'm thinking to replace const reference with `Option`; 
any suggestion?


- Klaus


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


On April 19, 2016, 3:31 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 19, 2016, 3:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-07-11 Thread Jie Yu

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




src/slave/main.cpp (lines 175 - 179)


Can we do that check in `add` function. `add` function supports an optional 
validate lambda to be passed in.


- Jie Yu


On April 19, 2016, 7:31 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 19, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-19 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [46298]

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 19, 2016, 7:31 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 19, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-19 Thread Klaus Ma


> On April 18, 2016, 5:47 p.m., Alexander Rukletsov wrote:
> > LGTM.
> > 
> > Do you want to document somewhere that if docker task will be run on the 
> > agent, this path should only contain symbols allowed for docker volumes 
> > (that's because we always create a docker volume for task's sandbox). Or do 
> > you want to do it in a separate patch?
> 
> Klaus Ma wrote:
> I'll submit another patch for that :).

@AlexR, I updated usage out for this in this patch.


- Klaus


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


On April 19, 2016, 3:31 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 19, 2016, 3:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-19 Thread Klaus Ma

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

(Updated April 19, 2016, 3:31 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Address AlexR's comments.


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs (updated)
-

  src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
  src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 

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


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-18 Thread Klaus Ma


> On April 18, 2016, 5:47 p.m., Alexander Rukletsov wrote:
> > LGTM.
> > 
> > Do you want to document somewhere that if docker task will be run on the 
> > agent, this path should only contain symbols allowed for docker volumes 
> > (that's because we always create a docker volume for task's sandbox). Or do 
> > you want to do it in a separate patch?

I'll submit another patch for that :).


- Klaus


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


On April 16, 2016, 4:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 16, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
>   src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-18 Thread Alexander Rukletsov

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



LGTM.

Do you want to document somewhere that if docker task will be run on the agent, 
this path should only contain symbols allowed for docker volumes (that's 
because we always create a docker volume for task's sandbox). Or do you want to 
do it in a separate patch?

- Alexander Rukletsov


On April 16, 2016, 8:29 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 16, 2016, 8:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
>   src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-16 Thread haosdent huang

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


Ship it!




Ship It!

- haosdent huang


On April 16, 2016, 8:29 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 16, 2016, 8:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
>   src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 46298: Rejected relative path agent work_dir.

2016-04-16 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [46298]

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 16, 2016, 8:29 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 16, 2016, 8:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
>   src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Review Request 46298: Rejected relative path agent work_dir.

2016-04-16 Thread Klaus Ma

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

Review request for mesos, Alexander Rukletsov and Jie Yu.


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs
-

  src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
  src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 

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


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma