Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 30, 2016, 11:54 a.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


Changes
---

fixed a typo


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
  src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
  src/master/main.cpp 871cf2d84eb13e429fe2d076cd7ce8d53c099944 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
  src/sched/sched.cpp 8380e48950f9f9c7919cea6de5236cec8cc1729d 
  src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
  src/slave/flags.cpp d30f39c216860c23e24d2d4064470c147c2824d2 
  src/slave/main.cpp f431e13703eafc003a1ffcbd0ee2247260e8708d 
  src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
  src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 

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


Testing
---

Manual testing with:
1. --module_dir
2. --modules
3. --module_dir in conjunction with --modules

The first two succeeded while the third failed as expected.


Thanks,

Kapil Arya

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 30, 2016, 11:22 a.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


Changes
---

rebased


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
  src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
  src/master/main.cpp 871cf2d84eb13e429fe2d076cd7ce8d53c099944 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
  src/sched/sched.cpp 8380e48950f9f9c7919cea6de5236cec8cc1729d 
  src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
  src/slave/flags.cpp d30f39c216860c23e24d2d4064470c147c2824d2 
  src/slave/main.cpp f431e13703eafc003a1ffcbd0ee2247260e8708d 
  src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
  src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 

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


Testing
---

Manual testing with:
1. --module_dir
2. --modules
3. --module_dir in conjunction with --modules

The first two succeeded while the third failed as expected.


Thanks,

Kapil Arya

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 30, 2016, 11:19 a.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  3rdparty/libprocess/CMakeLists.txt 6312e20ec0102edbef086bf61ce4256109ae8fa0 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
22ae4acdb3ff695799a8c69fa7282b0c57e1b37a 
  3rdparty/libprocess/configure.ac c8fcd35241c10829e7b2fa582491898589f0576f 
  3rdparty/libprocess/include/process/future.hpp 
455b750430ae04d16f610c24c0ea0feb8a033708 
  3rdparty/libprocess/include/process/http.hpp 
c180e6ebfbfc09e314eb6767e05cf22e546d977e 
  3rdparty/libprocess/include/process/io.hpp 
fed246bb96576b9aff247efedccc6af68b6dade3 
  3rdparty/libprocess/src/io.cpp d9c60754b1eaf83b1f5626a220f431cadeca541a 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
0c659dc00fe4fde78b2b57ebcc49cb47daf162d9 
  3rdparty/libprocess/src/tests/main.cpp 
a03dbc4ab46747003d8b11d22f2dc136293264ec 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 
  3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
  3rdparty/stout/include/Makefile.am 7f31582c176e653873402bd3f19b0c0195503d45 
  3rdparty/stout/include/stout/net.hpp 341d8ecea6f7e1244ad0027f110d6b0c69137667 
  3rdparty/stout/include/stout/os.hpp 53b00932693fba7cf6514da6a519269a904de345 
  3rdparty/stout/include/stout/os/access.hpp 
d87762a97152d55c58f6e54a9560c5fa0d051473 
  3rdparty/stout/include/stout/os/constants.hpp 
c71d52e1bc0433f9922f26a6eb486235bb9880d4 
  3rdparty/stout/include/stout/os/fsync.hpp 
bab337ed34f04111b12d266ba266a58b00c5e1ed 
  3rdparty/stout/include/stout/os/mkdir.hpp 
fe86864c8b480993c8f052f39b2fd3ece23798da 
  3rdparty/stout/include/stout/os/permissions.hpp 
1fa7614878cb0f58270824c88ab0013a9d9f14e9 
  3rdparty/stout/include/stout/os/posix/fork.hpp 
a906e4e03aac9ec9d95de62f905dcdbbba27c172 
  3rdparty/stout/include/stout/os/posix/fsync.hpp 
9a6bbf6c51b0a7d6085924733e33d8b4f1bbc1ac 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
63533ab4bb5c439b9921bc4476bf13e3baa16ef4 
  3rdparty/stout/include/stout/os/windows/fsync.hpp 
e1c4868b02b320906f446af1d9371374e90651bc 
  3rdparty/stout/include/stout/os/windows/killtree.hpp 
2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 
  3rdparty/stout/include/stout/os/windows/read.hpp 
cb0abf70307f0dbba0b8f68e884df199b0359186 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
e7f471b828d96bb5cd01823dd78c8570730712ea 
  3rdparty/stout/include/stout/os/windows/write.hpp 
705ad03ee58f9cc822ec1ed25fd10f39059717d4 
  3rdparty/stout/include/stout/path.hpp 
1b8326fd31562d82b6f17aa0945b9598184a9896 
  3rdparty/stout/include/stout/posix/net.hpp 
39b89828d350d83bf1d4b0badcd3e63eb9d1a630 
  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/protobuf.hpp 
7bf704d2d0dee423c386c961d5fe72779ad51b5a 
  3rdparty/stout/include/stout/windows.hpp 
a71e2f4965c982331c2e8fe4be70027b373a6516 
  3rdparty/stout/include/stout/windows/net.hpp 
1bed115cb848332bf9c31e455b2d001c173face9 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 
  CHANGELOG c778aeed24579d9c0ca880e7ebfa61a25f3cdd63 
  cmake/CompilationConfigure.cmake d3a5daee3c4c90712e920136bfe76fc9ab59ec45 
  docs/c++-style-guide.md a2e408f240611670fb2a72182b20fce9f0ebdcac 
  docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
  docs/committers.md e9f9dc520ab05e431051568bec9f6b8f19250960 
  docs/configuration.md edc6e26dd92e8577f05655ababfe8868ace80fca 
  docs/endpoints/index.md d22a56051b1335ef5097a8deea15efafbe5c2247 
  docs/endpoints/master/observe.md PRE-CREATION 
  docs/images/architecture-example.jpg e7a010c17779dc6c5cc0e785f3b0dac22da86b76 
  docs/images/architecture3.jpg aa82a4b42a23ab60dbea2c551df75e92fd375e24 
  docs/images/containerizer_isolator_api.png 
b84cd1dd082fc883b7f2b3b12458dc01e933d890 
  docs/images/ec_orphan_seqdiag.png 06de3a732414767a722e90c031d97a74150b 
  docs/images/ec_recover_seqdiag.png 08d4ca71a0c28bedc0436f5a01d9c506d1e32e54 
  docs/images/fetch_components.jpg 8d1ea9d395c4a586161508a78b59899cb41ea2ae 
  docs/images/networking-architecture.png 
4e767d4d8a8b23c73918765483dc1a98e3534926 
  docs/images/oversubscription-overview.jpg 
92ef297392910782a825dc1b3640b91d4dbe908c 
  docs/mesos-containerizer.md 1bd278392badcf34a305900491c1fd9b30c4420f 
  docs/multiple-disk.md 20b99e0e57360ad43804201b27e593d7ed48ce2c 
  

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Till Toenshoff]

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


Fix it, then Ship it!





src/module/manager.hpp (line 37)


Missing?

```
#include 
```

```
#include 
```



src/module/manager.cpp (line 28)


Insert blank line.



src/module/manager.cpp (line 30)


Insert blank line.



src/module/manager.cpp (line 369)


Do we really need this information on an INFO level - will the non coding 
user benefit from this or are possible error messages good enough already for 
determining the processed modules-dir?



src/module/manager.cpp (line 371)


s/std::string/string/



src/sched/sched.cpp (lines 1875 - 1877)


Why creating a local const in the first place?



src/tests/main.cpp (line 31)


Insert blank line.


- Till Toenshoff


On May 26, 2016, 5:04 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 26, 2016, 5:04 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
>   src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
>   src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
>   src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
>   src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 26, 2016, 1:04 p.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


Changes
---

Updated flags.


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
  src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
  src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
  src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
  src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
  src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
  src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
  src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
  src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 

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


Testing
---

Manual testing with:
1. --module_dir
2. --modules
3. --module_dir in conjunction with --modules

The first two succeeded while the third failed as expected.


Thanks,

Kapil Arya

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Till Toenshoff]

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



Looks good Kapil!

I would like to have tests and documentation updates as well before feeling 
comfortable to commit this - at least as [WIP] added here in a chain.


src/master/flags.cpp (line 407)


Could you please add a similar comment as you did for the `modules` flag 
above, hinting that we need to keep stuff in sync?

Same for all other references of that flag / help messages?


- Till Toenshoff


On May 20, 2016, 2:40 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 20, 2016, 2:40 a.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
>   src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
>   src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
>   src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
>   src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47123]

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 May 20, 2016, 2:40 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 20, 2016, 2:40 a.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
>   src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
>   src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
>   src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
>   src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 19, 2016, 10:40 p.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


Changes
---

Fixed compilation error.


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
  src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
  src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
  src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
  src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
  src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
  src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
  src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
  src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 

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


Testing
---

Manual testing with:
1. --module_dir
2. --modules
3. --module_dir in conjunction with --modules

The first two succeeded while the third failed as expected.


Thanks,

Kapil Arya

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

> On May 10, 2016, 1:27 a.m., Cody Maloney wrote:
> > src/module/manager.cpp, line 377
> > 
> >
> > Is there a semantic difference between loading modules one at a time 
> > vs. just passing them all at once to loadManifest?
> > 
> > I'm worried that the behavior here could differ slightly from the 
> > --modules flag which calls loadManifest with all the modules at once.

There is no semantic difference as of now. Even with the `--modules` flag, the 
modules are loaded one at a time, without any notion of interdependency. In 
future, if and when we add some other checks, say to find mutually exclusive 
modules, or interdependent modules, we might want to process the module 
manifests first (without actually loading the modules) and then load the 
modules at a later time. In that case, the change will be local to 
module/manager.cpp and so I wouldn't worry about it for now.


> On May 10, 2016, 1:27 a.m., Cody Maloney wrote:
> > src/module/manager.cpp, line 358
> > 
> >
> > Should document the sort order / load order based on filenames inside 
> > the doc changes to accompany this (I don't see it anywhere currently)

Added to the help message.


- Kapil


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


On May 19, 2016, 5:51 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 19, 2016, 5:51 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
>   src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
>   src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
>   src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
>   src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Cody Maloney]

> On May 10, 2016, 5:27 a.m., Cody Maloney wrote:
> >

I believe this covers the use cases we need. Be great to have a demo PR of it 
in action for:  https://github.com/dcos/dcos to validate that it does.


- Cody


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


On May 9, 2016, 5:10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 9, 2016, 5:10 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp e4cac1f8d688319c804e608b7229f458f779364a 
>   src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 
>   src/master/main.cpp 23149d5511d1556f1a885d01ea9380a9669fa8c5 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 4693d0dc09afc3ddbbf34e166579b6a6d71c3e38 
>   src/slave/flags.hpp 4fa3213545d4bd3525d85c3f71749f00f08dc998 
>   src/slave/flags.cpp 6fde51fc61cfcad61d4085c208bd2eca2eae8f14 
>   src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp c3ccf918c781bdb25b220c7ef3efa7d3b7c88c91 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Cody Maloney]

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




src/module/manager.cpp (line 357)


Should do the early exit:
`if (moduleManifests.isNone()) { return Nothing(); }`

That way the rest doesn't need to be indented.



src/module/manager.cpp (line 358)


Should document the sort order / load order based on filenames inside the 
doc changes to accompany this (I don't see it anywhere currently)



src/module/manager.cpp (line 377)


Is there a semantic difference between loading modules one at a time vs. 
just passing them all at once to loadManifest?

I'm worried that the behavior here could differ slightly from the --modules 
flag which calls loadManifest with all the modules at once.


- Cody Maloney


On May 9, 2016, 5:10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 9, 2016, 5:10 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp e4cac1f8d688319c804e608b7229f458f779364a 
>   src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 
>   src/master/main.cpp 23149d5511d1556f1a885d01ea9380a9669fa8c5 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 4693d0dc09afc3ddbbf34e166579b6a6d71c3e38 
>   src/slave/flags.hpp 4fa3213545d4bd3525d85c3f71749f00f08dc998 
>   src/slave/flags.cpp 6fde51fc61cfcad61d4085c208bd2eca2eae8f14 
>   src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp c3ccf918c781bdb25b220c7ef3efa7d3b7c88c91 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47123]

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 May 9, 2016, 5:10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 9, 2016, 5:10 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp e4cac1f8d688319c804e608b7229f458f779364a 
>   src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 
>   src/master/main.cpp 23149d5511d1556f1a885d01ea9380a9669fa8c5 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 4693d0dc09afc3ddbbf34e166579b6a6d71c3e38 
>   src/slave/flags.hpp 4fa3213545d4bd3525d85c3f71749f00f08dc998 
>   src/slave/flags.cpp 6fde51fc61cfcad61d4085c208bd2eca2eae8f14 
>   src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp c3ccf918c781bdb25b220c7ef3efa7d3b7c88c91 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 9, 2016, 1:10 p.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  src/master/flags.hpp e4cac1f8d688319c804e608b7229f458f779364a 
  src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 
  src/master/main.cpp 23149d5511d1556f1a885d01ea9380a9669fa8c5 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
  src/sched/sched.cpp 4693d0dc09afc3ddbbf34e166579b6a6d71c3e38 
  src/slave/flags.hpp 4fa3213545d4bd3525d85c3f71749f00f08dc998 
  src/slave/flags.cpp 6fde51fc61cfcad61d4085c208bd2eca2eae8f14 
  src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
  src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
  src/tests/main.cpp c3ccf918c781bdb25b220c7ef3efa7d3b7c88c91 

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


Testing
---

Manual testing with:
1. --module_dir
2. --modules
3. --module_dir in conjunction with --modules

The first two succeeded while the third failed as expected.


Thanks,

Kapil Arya

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Kapil Arya]

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

(Updated May 9, 2016, 1:05 p.m.)


Review request for mesos, Cody Maloney and Till Toenshoff.


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


Repository: mesos


Description
---

This allows the operator to use separate manifest JSON files for each
module.  Previously, one had to merge all module manifest files into a
single JSON file before passing on to the master/agent.


Diffs (updated)
-

  src/master/flags.hpp e4cac1f8d688319c804e608b7229f458f779364a 
  src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 
  src/master/main.cpp 23149d5511d1556f1a885d01ea9380a9669fa8c5 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
  src/sched/sched.cpp 4693d0dc09afc3ddbbf34e166579b6a6d71c3e38 
  src/slave/flags.hpp 4fa3213545d4bd3525d85c3f71749f00f08dc998 
  src/slave/flags.cpp 6fde51fc61cfcad61d4085c208bd2eca2eae8f14 
  src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
  src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
  src/tests/main.cpp c3ccf918c781bdb25b220c7ef3efa7d3b7c88c91 

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


Testing
---

Manual testing with:
1. --module_dir
2. --modules
3. --module_dir in conjunction with --modules

The first two succeeded while the third failed as expected.


Thanks,

Kapil Arya