----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68756/#review208759 -----------------------------------------------------------
src/resource_provider/local.hpp Lines 47 (patched) <https://reviews.apache.org/r/68756/#comment292904> This should be defined in `resource_provider/validation.hpp`, not here. Implementation in `resource_provider/validation.cpp`. I do understand why this is done here and it's good to have a single code path to dispatch from a resource provider type to its implementation. It would be great if we could separate this from the abstract factory that `LocalResourceProvider` provides instead of dispatching to static functions. But implementing a proper solution for this would add a lot of complexity (e.g. have the abstract factory create an instance of a factory that can create `LocalResourceProvider` implementation but also validate infos and probably do more in the future), so I'm okay with using static dispatch here. Though we should think of this again if there will be more disparch functions like this in the future. - Jan Schlicht On Sept. 19, 2018, 6:48 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68756/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2018, 6:48 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-9228 > https://issues.apache.org/jira/browse/MESOS-9228 > > > Repository: mesos > > > Description > ------- > > Each type of RP might have some specific validations for RP info. For > example, SLRP requires the `storage` field to be set. This patch makes > the local RP daemon to perform such validations when adding/updating > configs, so the `ADD_RESOURCE_PROVIDER_CONFIG` and > `UPDATE_RESOURCE_PROVIDER_CONFIG` calls can fail fast. > > > Diffs > ----- > > src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55 > src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29 > src/resource_provider/local.cpp 801e6c430ed91315d87f8a45b8f3ed128beca4fc > src/resource_provider/storage/provider.hpp > 5a371b19289c6e39fedd4cda65fa8be432d095e6 > src/resource_provider/storage/provider.cpp > 6475f653263337c381b6080695d09c49e5ea8fcf > src/slave/http.cpp f8199af50cf0b43bbbb8c29afe751fcd8949d7e8 > > > Diff: https://reviews.apache.org/r/68756/diff/1/ > > > Testing > ------- > > make check > > New tests added later in the chain. > > > Thanks, > > Chun-Hung Hsiao > >
