mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 65470: Added metrics for showing number of subscribed resource providers.
Date Fri, 02 Feb 2018 14:19:49 GMT

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


Fix it, then Ship it!





src/resource_provider/manager.cpp
Lines 75 (patched)
<https://reviews.apache.org/r/65470/#comment276496>

    If this section is sorted like includes, this should go right after the first `process::http`
section just below.



src/resource_provider/manager.cpp
Lines 200-201 (original), 218-219 (patched)
<https://reviews.apache.org/r/65470/#comment276509>

    Not yours, but we should pull these onto the line of the last parameter like elsewhere.



src/resource_provider/manager.cpp
Lines 772 (patched)
<https://reviews.apache.org/r/65470/#comment276510>

    At some point we'll move the prefix to a constant somewhere.



src/tests/resource_provider_manager_tests.cpp
Lines 1322-1323 (patched)
<https://reviews.apache.org/r/65470/#comment276511>

    `StartMaster` will automatically pass default `masterFlags` if none are given. Let's remove
them here since we do not need them in the test.



src/tests/resource_provider_manager_tests.cpp
Lines 1352 (patched)
<https://reviews.apache.org/r/65470/#comment276512>

    `s/subscribed1/subscribed/g`
    
    Here and below.



src/tests/resource_provider_manager_tests.cpp
Lines 1365 (patched)
<https://reviews.apache.org/r/65470/#comment276513>

    `map`'s `operator[]` is not `const` which makes it unclear whether the test unknowingly
modifies the test data. I'd suggest to make `snapshot` `const` and then use `at` here. We
should also add an assertion to the values contain that key just before attempting to access
it.


- Benjamin Bannier


On Feb. 2, 2018, 1:27 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65470/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Greg Mann.
> 
> 
> Bugs: MESOS-8527
>     https://issues.apache.org/jira/browse/MESOS-8527
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added metrics for showing number of subscribed resource providers.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/manager.cpp 5d064bff76d5f18d85e6c050adf9edbc26107c07 
>   src/tests/resource_provider_manager_tests.cpp 2944b066f8bfcbe328469940eb0e17b9b2809f63

> 
> 
> Diff: https://reviews.apache.org/r/65470/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message