Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Gilbert Song]

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

(Updated Sept. 7, 2016, 12:01 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Implemented recursive helper method findContainerDir for provisioner.


Diffs
-

  src/slave/containerizer/mesos/provisioner/paths.hpp 
9829d6b52c8547ae22297a5bc47852ce5a219e4c 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Qian Zhang]

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




src/slave/containerizer/mesos/provisioner/paths.cpp (line 106)


I think it may be better to make this method just return `Try` 
rather than `Try` to simplify the work of the caller. If we 
return `Try`, then the caller need to check 1) 
`result.isError()` and 2) `result.get().isSome()`. But if we just return 
`Try`, then the caller only needs to check 1).



src/slave/containerizer/mesos/provisioner/paths.cpp (line 113)


Shouldn't we return an `Error` in this case?



src/slave/containerizer/mesos/provisioner/paths.cpp (line 127)


Do we have to call `findContainerDir()` in a recursive way to find out the 
sub-container directory?
I think we can actually figure out this is a top level container or 
sub-container by checking `containerId.has_parent()`, so if it is a 
sub-container, we can directly locate its parent container's directory first 
(`getContainerDir(provisionerDir, containerId.parent())`), and then find out 
the sub-container directory under it.


- Qian Zhang


On Aug. 30, 2016, 5:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51358/
> ---
> 
> (Updated Aug. 30, 2016, 5:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recursive helper method findContainerDir for provisioner.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.hpp 
> 9829d6b52c8547ae22297a5bc47852ce5a219e4c 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> 86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 
> 
> Diff: https://reviews.apache.org/r/51358/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Guangya Liu]

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




src/slave/containerizer/mesos/provisioner/paths.cpp (line 111)


kill this



src/slave/containerizer/mesos/provisioner/paths.cpp (lines 132 - 133)


How about putting the `container directory path` into the error message?


- Guangya Liu


On 八月 29, 2016, 9:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51358/
> ---
> 
> (Updated 八月 29, 2016, 9:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recursive helper method findContainerDir for provisioner.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.hpp 
> 9829d6b52c8547ae22297a5bc47852ce5a219e4c 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> 86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 
> 
> Diff: https://reviews.apache.org/r/51358/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Joseph Wu]

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




src/slave/containerizer/mesos/provisioner/paths.cpp (line 122)


Shouldn't you check if the entries are directories here?



src/slave/containerizer/mesos/provisioner/paths.cpp (lines 131 - 138)


You should change the return type to a `Result`.


- Joseph Wu


On Aug. 29, 2016, 2:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51358/
> ---
> 
> (Updated Aug. 29, 2016, 2:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recursive helper method findContainerDir for provisioner.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.hpp 
> 9829d6b52c8547ae22297a5bc47852ce5a219e4c 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> 86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 
> 
> Diff: https://reviews.apache.org/r/51358/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Implemented recursive helper method findContainerDir for provisioner.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/paths.hpp 
9829d6b52c8547ae22297a5bc47852ce5a219e4c 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Gilbert Song]

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

(Updated Aug. 24, 2016, 3:14 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, and 
Kevin Klues.


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


Repository: mesos


Description
---

Implemented recursive helper method findContainerDir for provisioner.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/paths.hpp 
9829d6b52c8547ae22297a5bc47852ce5a219e4c 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Gilbert Song]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, and 
Kevin Klues.


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


Repository: mesos


Description
---

Implemented recursive helper method findContainerDir for provisioner.


Diffs
-

  src/slave/containerizer/mesos/provisioner/paths.hpp 
9829d6b52c8547ae22297a5bc47852ce5a219e4c 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing
---

make check


Thanks,

Gilbert Song