You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Bernd Mathiske <be...@mesosphere.io> on 2015/01/03 18:36:42 UTC

Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

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

Review request for mesos, Adam B and Benjamin Hindman.

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

Repository: mesos-git

Description
-------

More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests has been eliminated. It was not deemed essential to debugging.

Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

Remodelled some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.

Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.

Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.

The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.

Diffs
-----

include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721
include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437
src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88
src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7
src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829
src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748
src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032
src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52
src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef
src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439
src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7

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

Testing
-------

make check

Thanks,

Bernd Mathiske

Re: Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

Posted by Bernd Mathiske <be...@mesosphere.io>.


> On Jan. 6, 2015, 10:34 a.m., Timothy Chen wrote:
> > include/mesos/mesos.proto, line 211
> > <https://reviews.apache.org/r/29560/diff/2/?file=807840#file807840line211>
> >
> >     cache seems to be the verb to save the uri

I believe that the preceding "extract" was a mistake. Should have been "extracting". There isn't really any other precedence for a verb that I could find, but there are lots of adjectives in use and the closest verb form to that is not infinitive or imperative, but present participle (-ing form).


> On Jan. 6, 2015, 10:34 a.m., Timothy Chen wrote:
> > include/mesos/fetcher/fetcher.proto, line 41
> > <https://reviews.apache.org/r/29560/diff/2/?file=807839#file807839line41>
> >
> >     This requires the user to know that the URI is already in the cache right? Why not just leave the last option to be DOWNLOAD_AND_INSTALL?
> >     
> >     And what does install mean? We used to fetch and extract all uris, does extracting means install?

The user does not (need to) track what is in the cache and what not. The MesosFetcherProcess does, though, and it communicates these actions to the external fetcher program. 

I could change the ordering of these enums if so desired, but why? "Downloading and installing" happens for the first fetch, "only installing" for all subsequent ones for the same URI, as long as the item is in the cache.

Extracting is somewhat orthogonal to all that. There are choices when exactly this should happen if so selected, either during downloading or during installation. I am opting for the latter as default behavior.

A better name for "install" would be welcome. What it means: get something from the cache and place it into the work dir as if it were downloaded directly from the source.


> On Jan. 6, 2015, 10:34 a.m., Timothy Chen wrote:
> > src/slave/containerizer/fetcher.cpp, line 508
> > <https://reviews.apache.org/r/29560/diff/2/?file=807844#file807844line508>
> >
> >     Why not just open getCacheFile, and if they need the file name it will be part of the CacheFile struct? 
> >     
> >     It seems more cleaner that way. And even if we want to expose a getCacheFilename method, it should at least return option instead of empty string

Let's look at this again in the next iteration (MESOS-2057). I deleted this so far unused method for now.


- Bernd


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


On Jan. 6, 2015, 7:50 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 7:50 a.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests has been eliminated. It was not deemed essential to debugging. 
> 
> Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodelled some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

Posted by Timothy Chen <tn...@apache.org>.

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



include/mesos/fetcher/fetcher.proto
<https://reviews.apache.org/r/29560/#comment110574>

    We push these enum into local scope unless this is shared. Fetch Action seems very fetcher specific, so I think we should put this into FetchInfo



include/mesos/fetcher/fetcher.proto
<https://reviews.apache.org/r/29560/#comment110557>

    This requires the user to know that the URI is already in the cache right? Why not just leave the last option to be DOWNLOAD_AND_INSTALL?
    
    And what does install mean? We used to fetch and extract all uris, does extracting means install?



include/mesos/mesos.proto
<https://reviews.apache.org/r/29560/#comment110558>

    cache seems to be the verb to save the uri



src/slave/containerizer/fetcher.cpp
<https://reviews.apache.org/r/29560/#comment110571>

    All our defines are usually before all declartions



src/slave/containerizer/fetcher.cpp
<https://reviews.apache.org/r/29560/#comment110572>

    stringify



src/slave/containerizer/fetcher.cpp
<https://reviews.apache.org/r/29560/#comment110573>

    Why not just open getCacheFile, and if they need the file name it will be part of the CacheFile struct? 
    
    It seems more cleaner that way. And even if we want to expose a getCacheFilename method, it should at least return option instead of empty string


- Timothy Chen


On Jan. 6, 2015, 3:50 p.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests has been eliminated. It was not deemed essential to debugging. 
> 
> Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodelled some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

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


Patch looks great!

Reviews applied: [29560]

All tests passed.

- Mesos ReviewBot


On Jan. 6, 2015, 3:50 p.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests has been eliminated. It was not deemed essential to debugging. 
> 
> Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodelled some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

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


Patch looks great!

Reviews applied: [29560]

All tests passed.

- Mesos ReviewBot


On Jan. 7, 2015, 11:32 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2015, 11:32 a.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests has been eliminated. It was not deemed essential to debugging. 
> 
> Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodelled some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Bernd Mathiske <be...@mesosphere.io>.

(Updated Jan. 17, 2015, 3:35 a.m.)

Review request for mesos, Adam B, Benjamin Hindman, and Timothy Chen.

Changes
-------

+tnachen (Tim)

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

Repository: mesos-git

Description
-------

More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests is eliminated. It is not deemed essential to debugging.

Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

Remodels some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.

We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).

Diffs
-----

include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721
include/mesos/mesos.proto 5007c0c58c5573ef19163c2c58cc3a7c5b555f46
src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89
src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88
src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7
src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829
src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748
src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032
src/slave/containerizer/mesos/containerizer.cpp 0bcf5ce7cfab470cabd3af3535344d19cb33b1c8
src/slave/flags.hpp f1b8dfbb7391167b67a9498561742aa9ab9089a6
src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439
src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7

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

Testing
-------

make check

Thanks,

Bernd Mathiske

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Bernd Mathiske <be...@mesosphere.io>.

(Updated Jan. 14, 2015, 2:58 a.m.)

Review request for mesos, Adam B and Benjamin Hindman.

Changes
-------

Pilot error. Grabbed the wrong diff file last time. This one should work.

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

Repository: mesos-git

Description
-------

Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).

Diffs (updated)
-----

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

Testing
-------

make check

Thanks,

Bernd Mathiske

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Bernd Mathiske <be...@mesosphere.io>.

(Updated Jan. 14, 2015, 2:34 a.m.)

Review request for mesos, Adam B and Benjamin Hindman.

Changes
-------

Addressed Alex' review, reshuffled FetcherInfo and replaced CommandInfo with the mere URIs. The rest of CommandInfo is never needed by the external fetcher program.

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

Repository: mesos-git

Description
-------

Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).

Diffs (updated)
-----

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

Testing
-------

make check

Thanks,

Bernd Mathiske

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Bernd Mathiske <be...@mesosphere.io>.


> On Jan. 13, 2015, 7:55 a.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/fetcher.cpp, lines 186-187
> > <https://reviews.apache.org/r/29560/diff/3/?file=808632#file808632line186>
> >
> >     `fetcher_cache_dir` is not optional, while `cache_directory` is. Maybe we should consider making the former optional as well?
> 
> Bernd Mathiske wrote:
>     I will make cache_directory required.
> 
> Alexander Rukletsov wrote:
>     This may be not the best solution: once the field is marked as required, it will stick forever and cannot be changed back to optional. Since cache directory doesn't feel like an absolutely "must have" (the caching may be not used, according to your comment below), maybe it makes sense to keep the protobuf field optional?

Agreed. It will be allowed to omit the field in case a fetcher run never uses the cache. Back to making cahce_directory optional. However, the slave cannot foresee that all fetcher runs will comply with this. Therefore specifying the cache dir remains mandatory, if only by untouched default value.


- Bernd


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


On Jan. 14, 2015, 2:58 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 2:58 a.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests is eliminated. It is not deemed essential to debugging. 
> 
> Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodels some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 5007c0c58c5573ef19163c2c58cc3a7c5b555f46 
>   src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 0bcf5ce7cfab470cabd3af3535344d19cb33b1c8 
>   src/slave/flags.hpp f1b8dfbb7391167b67a9498561742aa9ab9089a6 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Alexander Rukletsov <al...@mesosphere.io>.


> On Jan. 13, 2015, 3:55 p.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/fetcher.cpp, lines 186-187
> > <https://reviews.apache.org/r/29560/diff/3/?file=808632#file808632line186>
> >
> >     `fetcher_cache_dir` is not optional, while `cache_directory` is. Maybe we should consider making the former optional as well?
> 
> Bernd Mathiske wrote:
>     I will make cache_directory required.

This may be not the best solution: once the field is marked as required, it will stick forever and cannot be changed back to optional. Since cache directory doesn't feel like an absolutely "must have" (the caching may be not used, according to your comment below), maybe it makes sense to keep the protobuf field optional?


- Alexander


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


On Jan. 14, 2015, 10:58 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 10:58 a.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests is eliminated. It is not deemed essential to debugging. 
> 
> Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodels some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 5007c0c58c5573ef19163c2c58cc3a7c5b555f46 
>   src/launcher/fetcher.cpp fed0105946da579a38357a30e7ae56e646e05b89 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 0bcf5ce7cfab470cabd3af3535344d19cb33b1c8 
>   src/slave/flags.hpp f1b8dfbb7391167b67a9498561742aa9ab9089a6 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Bernd Mathiske <be...@mesosphere.io>.


> On Jan. 13, 2015, 7:55 a.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/fetcher.cpp, lines 186-187
> > <https://reviews.apache.org/r/29560/diff/3/?file=808632#file808632line186>
> >
> >     `fetcher_cache_dir` is not optional, while `cache_directory` is. Maybe we should consider making the former optional as well?

I will make cache_directory required.


> On Jan. 13, 2015, 7:55 a.m., Alexander Rukletsov wrote:
> > src/slave/containerizer/fetcher.cpp, line 495
> > <https://reviews.apache.org/r/29560/diff/3/?file=808632#file808632line495>
> >
> >     It looks like if the `Flags::fetcher_cache_dir` points to a non-existent dir, we return a path that doesn't exist. Can it be an issue?

At some point I will add code that creates the dir. Not an issue as long as with these earlier patches users still only exercise the legacy FETCH action without using the cache.


> On Jan. 13, 2015, 7:55 a.m., Alexander Rukletsov wrote:
> > src/slave/flags.hpp, line 449
> > <https://reviews.apache.org/r/29560/diff/3/?file=808635#file808635line449>
> >
> >     Do we require this parameter to be always present? If no, let's make it `Optional<>`, if yes, maybe it makes sense to check that the directory is present and accessible?

See above. The dir will be created on demand when exercising the cache. If that fails, fetching fails. But if caching is never used, the dir is not attempted to be created and no harm is done. This slave parameter/flag needs to be always present, because the slave cannot foresee that no task will ever use the fetcher cache. On the contrary, it needs to be prepared for tasks wanting to use the cache.


- Bernd


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


On Jan. 13, 2015, 5:35 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 5:35 a.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests is eliminated. It is not deemed essential to debugging. 
> 
> Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodels some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

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



src/slave/containerizer/fetcher.cpp
<https://reviews.apache.org/r/29560/#comment111959>

    `fetcher_cache_dir` is not optional, while `cache_directory` is. Maybe we should consider making the former optional as well?



src/slave/containerizer/fetcher.cpp
<https://reviews.apache.org/r/29560/#comment111961>

    It looks like if the `Flags::fetcher_cache_dir` points to a non-existent dir, we return a path that doesn't exist. Can it be an issue?



src/slave/flags.hpp
<https://reviews.apache.org/r/29560/#comment111958>

    Do we require this parameter to be always present? If no, let's make it `Optional<>`, if yes, maybe it makes sense to check that the directory is present and accessible?


- Alexander Rukletsov


On Jan. 13, 2015, 1:35 p.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29560/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 1:35 p.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2069
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> More refactoring, resolving overloading of the Fetcher::fetch() and Fetcher::run() methods. To achieve this, the capability to redirect fetcher stderr/out from tests is eliminated. It is not deemed essential to debugging. 
> 
> Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).
> 
> Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now. 
> 
> Remodels some fetcher tests to be more "black-box-like", calling the fetcher process more indirectly, because essentially a lot of code would just be copied from the fetcher implementation to the tests, otherwise. Now all fetcher tests follow the same, simpler pattern.
> 
> We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).
> 
> Note some TODO(bernd-mesos) items. Where these appear, MESOS-2057 will fill in the missing details. For example, there is good reason for Fetcher::_fetch(). It will call run() and checkRunStatus() then.
> 
> The fetcher needs to get a hold of the slave ID from somewhere. The chosen approach here is to hand it down as a parameter, because this follows prevalent patterns. However, in principle this leaves room for misuse in that the same fetcher could potentially be used with two different slaves (which would cause various bugs). Alternatively, one could store the slave ID in the fetcher at some point. But that would be subject to a race. So neither approach is ideal.
> 
> 
> Diffs
> -----
> 
>   include/mesos/fetcher/fetcher.proto facb87b92bf3194516f636dcc348e136af537721 
>   include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 
>   src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88 
>   src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7 
>   src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 5993670f7899233efa1e6acef4b0c7856e32f748 
>   src/slave/containerizer/mesos/containerizer.hpp 802988c90ac872b0cefa5e28f06e6fec98e8d032 
>   src/slave/containerizer/mesos/containerizer.cpp 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 
>   src/slave/flags.hpp 670997dc3a702cd5edf33f2e5824c5e4dfe4ecef 
>   src/tests/docker_containerizer_tests.cpp 2105ae2c410f01e7e0d10241d5c00df143fd3439 
>   src/tests/fetcher_tests.cpp 8c0b0757eb388f1684d8b94393983f1844a769a7 
> 
> Diff: https://reviews.apache.org/r/29560/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 29560: Added basic cache data structures to the fetcher process.

Posted by Bernd Mathiske <be...@mesosphere.io>.

(Updated Jan. 13, 2015, 5:35 a.m.)

Review request for mesos, Adam B and Benjamin Hindman.

Changes
-------

More compliant with common summary and description formats.

Summary (updated)
-----------------

Added basic cache data structures to the fetcher process.

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

Repository: mesos-git

Description (updated)
-------

Extends the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduces essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

We skip implementing cache functionality in launcher/fetcher.cpp for now. It will come in a separate, dependent patch (https://reviews.apache.org/r/29809/).

Diffs
-----

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

Testing
-------

make check

Thanks,

Bernd Mathiske

Re: Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

Posted by Bernd Mathiske <be...@mesosphere.io>.

(Updated Jan. 7, 2015, 3:32 a.m.)

Review request for mesos, Adam B and Benjamin Hindman.

Changes
-------

Addressed Tim's review. Thanks, Tim!

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

Repository: mesos-git

Description
-------

Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.

Diffs (updated)
-----

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

Testing
-------

make check

Thanks,

Bernd Mathiske

Re: Review Request 29560: MESOS-2069: Basic fetcher cache functionality.

Posted by Bernd Mathiske <be...@mesosphere.io>.

(Updated Jan. 6, 2015, 7:50 a.m.)

Review request for mesos, Adam B and Benjamin Hindman.

Changes
-------

Added missing method.

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

Repository: mesos-git

Description
-------

Extended the FetcherInfo proto, the parameter aggregate for the fetcher program. Introduced essential parts and pieces, e.g. cache directory, fetcher actions, to be built out later (MESOS-2057).

Moved construction of the fetcher parameters (now FetcherInfo) into the fetch method and thereupon eliminated all fetcher environment tests, since building the environment is trivial now.

Skipped implementing cache functionality in launcher/fetcher.cpp for now, since it won't do anything until MESOS-2057, which is next, and both reviews are more balanced in size this way.

Diffs (updated)
-----

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

Testing
-------

make check

Thanks,

Bernd Mathiske