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 70247: Added an "unversioned" `VolumeCapability`.
Date Tue, 26 Mar 2019 17:27:07 GMT


> On March 20, 2019, 3:09 p.m., Benjamin Bannier wrote:
> > include/mesos/csi/compat.proto
> > Lines 1 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132974#file2132974line1>
> >
> >     I don't think these definitions need to be public as they never leave the agent.
Let's just add them to `src/csi/` or sim?
> 
> Chun-Hung Hsiao wrote:
>     The definition will be used in the disk profile adaptor. This can be discussed, but
I personally feel that it's better if our module interface doesn't depend on a third-party
protobuf so we can control our own backward compatibility.

Makes sense, dropping.


> On March 20, 2019, 3:09 p.m., Benjamin Bannier wrote:
> > src/csi/compat.cpp
> > Lines 25-61 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132977#file2132977line25>
> >
> >     `return google::protobuf::util::MessageDifferencer::Equals(left, right)`?
> 
> Chun-Hung Hsiao wrote:
>     The reason we generally don't use `MessageDifferencer` is that we probably want to
define our own equality semantics, for example, the "ordering may not matter" constraint here
(although it's not implemented). Also, do we want to compare unknown fields?
>     
>     If we want a complete message equality check, then yes I'll just use `MessageDifferencer`.

I'd much prefer a world where each of such functions either forwards to a proto message differencer
_or_ explicitly declares why it deviates from that implementation. Could you update the code
for that?


- Benjamin


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


On March 26, 2019, 6:10 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
> 
> (Updated March 26, 2019, 6:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9625
>     https://issues.apache.org/jira/browse/MESOS-9625
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
> 
> 1. The unversioned `VolumeCapability` parsed from a versioned one should
>    be able to used to reconstruct the original versioned
>    `VolumeCapability`, and can be upgraded/downgraded to a different
>    CSI version and preserve as much semantics as possible.
> 
> 2. If an backward-incompatible change is introduced in future CSI
>    `VolumeCapability`, the unversioned `VolumeCapability` can provide a
>    way to do a backward compatible upgrade.
> 
> 
> Diffs
> -----
> 
>   include/mesos/csi/types.hpp PRE-CREATION 
>   include/mesos/csi/types.proto PRE-CREATION 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
>   src/csi/types.cpp PRE-CREATION 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc 
> 
> 
> Diff: https://reviews.apache.org/r/70247/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


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