----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70314/#review214091 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/storage/provider.cpp Line 1 (original), 1 (patched) <https://reviews.apache.org/r/70314/#comment300260> This patch should contain a test of introduced behavior. src/resource_provider/storage/provider.cpp Lines 2646 (patched) <https://reviews.apache.org/r/70314/#comment300256> Add a comment here documenting what this branch models. src/resource_provider/storage/provider.cpp Lines 2702 (patched) <https://reviews.apache.org/r/70314/#comment300258> All this local checking of capabilities at call sites instead of in `call` is getting cumbersome and error-prone. Would be great to clean that up eventually by moving it to a single place. src/resource_provider/storage/provider.cpp Lines 2725 (patched) <https://reviews.apache.org/r/70314/#comment300257> Great, should have been here all along. src/resource_provider/storage/provider.cpp Lines 3202-3203 (original), 3194-3196 (patched) <https://reviews.apache.org/r/70314/#comment300259> Let's use a `switch` now that we have three cases here. src/resource_provider/storage/provider.cpp Lines 3237-3239 (original), 3235-3237 (patched) <https://reviews.apache.org/r/70314/#comment300255> No quotes around `resource`; let's also fix the wrapping so that related entities are on the same line (`"resource provider"` and `info.id()`; `"resource"` and `resource`; opening and closing quotes). ``` LOG(INFO) << "Reconciling storage pools for resource provider " << info.id() << " after resource '" << resource << "' has been freed"; - Benjamin Bannier On March 27, 2019, 4:45 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70314/ > ----------------------------------------------------------- > > (Updated March 27, 2019, 4:45 a.m.) > > > Review request for mesos, Benjamin Bannier and Jie Yu. > > > Bugs: MESOS-9540 > https://issues.apache.org/jira/browse/MESOS-9540 > > > Repository: mesos > > > Description > ------- > > SLRP now accepts `DESTROY_DISK` on `RAW` disk resources with source IDs. > If the backed CSI plugin does have the `CREATE_DELETE_VOLUME` controller > capability, this operation will be a no-op; otherwise the underlying CSI > volume will be deprovisioned. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > 2711503cdb58cb9b34af8c9fad0908c5f788a2af > > > Diff: https://reviews.apache.org/r/70314/diff/2/ > > > Testing > ------- > > make check > > The new code path is tested later in this chain. > > > Thanks, > > Chun-Hung Hsiao > >
