Re: Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.

2017-12-18 Thread Jie Yu 

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


Fix it, then Ship it!





src/resource_provider/volume_profile.cpp
Lines 110 (patched)


2 lines apart


- Jie Yu


On Dec. 19, 2017, 3:08 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64612/
> ---
> 
> (Updated Dec. 19, 2017, 3:08 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
> 
> 
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds an agent flag to choose the VolumeProfileAdaptor module
> to load and passes this information down to the storage resource
> providers. A single module can potentially be used by multiple
> storage resource providers.
> 
> We choose to pass the module along as a global variable because the
> module is not needed by many of the components in the injection path.
> Specifically, the Storage Local Resource Provider will need the module,
> but its parents, the Local Resource Provider and Resource Provider
> Daemons will not.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/volume_profile.hpp PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> 158b6b408002209aa9a79a6772da30c984aad61a 
>   src/resource_provider/volume_profile.cpp PRE-CREATION 
>   src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
>   src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 
>   src/slave/slave.hpp 75cc5836e39b2413eaefc5a5c9e905134db83743 
>   src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 
> 
> 
> Diff: https://reviews.apache.org/r/64612/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> Basically just a plumbing change to pass the module into the SLRP.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.

2017-12-18 Thread Joseph Wu 

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

(Updated Dec. 18, 2017, 7:08 p.m.)


Review request for mesos, Chun-Hung Hsiao and Jie Yu.


Changes
---

Changed a global var to a POD type.  Added some comments.


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


Repository: mesos


Description
---

This adds an agent flag to choose the VolumeProfileAdaptor module
to load and passes this information down to the storage resource
providers. A single module can potentially be used by multiple
storage resource providers.

We choose to pass the module along as a global variable because the
module is not needed by many of the components in the injection path.
Specifically, the Storage Local Resource Provider will need the module,
but its parents, the Local Resource Provider and Resource Provider
Daemons will not.


Diffs (updated)
-

  include/mesos/resource_provider/volume_profile.hpp PRE-CREATION 
  src/resource_provider/storage/provider.cpp 
158b6b408002209aa9a79a6772da30c984aad61a 
  src/resource_provider/volume_profile.cpp PRE-CREATION 
  src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
  src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 
  src/slave/slave.hpp 75cc5836e39b2413eaefc5a5c9e905134db83743 
  src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 


Diff: https://reviews.apache.org/r/64612/diff/3/

Changes: https://reviews.apache.org/r/64612/diff/2-3/


Testing
---

make check

Basically just a plumbing change to pass the module into the SLRP.


Thanks,

Joseph Wu

Re: Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.

2017-12-18 Thread Joseph Wu 


> On Dec. 15, 2017, 3:26 p.m., Jie Yu wrote:
> > src/resource_provider/local.cpp
> > Lines 39 (patched)
> > 
> >
> > It's weird that LocalResourceProvider needs to be aware 
> > `volumeProfileAdaptor` which is very StorageLocalResourceProvider specific.
> > 
> > It's more appropriate to instantiate that in 
> > StorageLocalResourceProvider and make it a singleton.
> > 
> > probably passing the agent flags all the way to this method (or use 
> > `Parameters` to pass in provider specifc parameters).

I've changed the injection method.  I did it the way I did ('create', 'get', 
and 'set' methods) because tests may instantiate different varieties of the 
module in tests, so just having a `once` is not sufficient to allow this.


- Joseph


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


On Dec. 18, 2017, 6:35 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64612/
> ---
> 
> (Updated Dec. 18, 2017, 6:35 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
> 
> 
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds an agent flag to choose the VolumeProfileAdaptor module
> to load and passes this information down to the storage resource
> providers. A single module can potentially be used by multiple
> storage resource providers.
> 
> We choose to pass the module along as a global variable because the
> module is not needed by many of the components in the injection path.
> Specifically, the Storage Local Resource Provider will need the module,
> but its parents, the Local Resource Provider and Resource Provider
> Daemons will not.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/volume_profile.hpp PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 
> 158b6b408002209aa9a79a6772da30c984aad61a 
>   src/resource_provider/volume_profile.cpp PRE-CREATION 
>   src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
>   src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 
>   src/slave/slave.hpp 75cc5836e39b2413eaefc5a5c9e905134db83743 
>   src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 
> 
> 
> Diff: https://reviews.apache.org/r/64612/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Basically just a plumbing change to pass the module into the SLRP.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.

2017-12-18 Thread Joseph Wu 

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

(Updated Dec. 18, 2017, 6:35 p.m.)


Review request for mesos, Chun-Hung Hsiao and Jie Yu.


Changes
---

Changed the injection method for the module.  This involves much fewer 
interface changes, but is also unlike most of our other patterns.


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


Repository: mesos


Description (updated)
---

This adds an agent flag to choose the VolumeProfileAdaptor module
to load and passes this information down to the storage resource
providers. A single module can potentially be used by multiple
storage resource providers.

We choose to pass the module along as a global variable because the
module is not needed by many of the components in the injection path.
Specifically, the Storage Local Resource Provider will need the module,
but its parents, the Local Resource Provider and Resource Provider
Daemons will not.


Diffs (updated)
-

  include/mesos/resource_provider/volume_profile.hpp PRE-CREATION 
  src/resource_provider/storage/provider.cpp 
158b6b408002209aa9a79a6772da30c984aad61a 
  src/resource_provider/volume_profile.cpp PRE-CREATION 
  src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
  src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 
  src/slave/slave.hpp 75cc5836e39b2413eaefc5a5c9e905134db83743 
  src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 


Diff: https://reviews.apache.org/r/64612/diff/2/

Changes: https://reviews.apache.org/r/64612/diff/1-2/


Testing
---

make check

Basically just a plumbing change to pass the module into the SLRP.


Thanks,

Joseph Wu

Re: Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.

2017-12-15 Thread Jie Yu 

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




src/resource_provider/local.cpp
Lines 39 (patched)


It's weird that LocalResourceProvider needs to be aware 
`volumeProfileAdaptor` which is very StorageLocalResourceProvider specific.

It's more appropriate to instantiate that in StorageLocalResourceProvider 
and make it a singleton.

probably passing the agent flags all the way to this method (or use 
`Parameters` to pass in provider specifc parameters).


- Jie Yu


On Dec. 14, 2017, 1:15 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64612/
> ---
> 
> (Updated Dec. 14, 2017, 1:15 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
> 
> 
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds an agent flag to choose the VolumeProfileAdaptor module
> to load and passes this information down to the storage resource
> providers. A single module can potentially be used by multiple
> storage resource providers.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.hpp a6d0013fa3645fd2b705351a86679f7fba13e7e3 
>   src/resource_provider/daemon.cpp f160a87c37d975d632db05694ca7b59f1d275821 
>   src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29 
>   src/resource_provider/local.cpp ae23c2000b7941aad92d3b83e4e905bdf042ebe8 
>   src/resource_provider/storage/provider.hpp 
> 5a371b19289c6e39fedd4cda65fa8be432d095e6 
>   src/resource_provider/storage/provider.cpp 
> f00fe58e12dcaa911ab3d05c3d7719c65a1484fb 
>   src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
>   src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 
> 
> 
> Diff: https://reviews.apache.org/r/64612/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> Basically just a plumbing change to pass the module into the SLRP.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.

2017-12-14 Thread Joseph Wu 

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

Review request for mesos, Chun-Hung Hsiao and Jie Yu.


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


Repository: mesos


Description
---

This adds an agent flag to choose the VolumeProfileAdaptor module
to load and passes this information down to the storage resource
providers. A single module can potentially be used by multiple
storage resource providers.


Diffs
-

  src/resource_provider/daemon.hpp a6d0013fa3645fd2b705351a86679f7fba13e7e3 
  src/resource_provider/daemon.cpp f160a87c37d975d632db05694ca7b59f1d275821 
  src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29 
  src/resource_provider/local.cpp ae23c2000b7941aad92d3b83e4e905bdf042ebe8 
  src/resource_provider/storage/provider.hpp 
5a371b19289c6e39fedd4cda65fa8be432d095e6 
  src/resource_provider/storage/provider.cpp 
f00fe58e12dcaa911ab3d05c3d7719c65a1484fb 
  src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
  src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 


Diff: https://reviews.apache.org/r/64612/diff/1/


Testing
---

make check

Basically just a plumbing change to pass the module into the SLRP.


Thanks,

Joseph Wu