mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 72661: Updated `CSIPluginInfo` for supporting 3rd party CSI plugins.
Date Tue, 21 Jul 2020 16:50:37 GMT


> On July 16, 2020, 11:22 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Line 1106 (original), 1106 (patched)
> > <https://reviews.apache.org/r/72661/diff/5/?file=2235882#file2235882line1106>
> >
> >     I'm not sure that we should mark this as deprecated, since it's still useful
for the LVM driver?
> 
> Qian Zhang wrote:
>     Yeah, it is still userful for the LVM driver, but will we continue to support it
in long term?

I would propose that we don't tie this patch to the decision of whether or not we want to
continue supporting it, I'm not sure what the answer for that is?


> On July 16, 2020, 11:22 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 1126-1129 (patched)
> > <https://reviews.apache.org/r/72661/diff/5/?file=2235882#file2235882line1126>
> >
> >     Should we define this next to `CSIPluginContainerInfo` for consistency? Also,
can we move the `Service` message up to the top level of the namespace without breaking the
API? Would be nice for `Service` to sit higher now that it's used by two messages; but the
name might be too generic for the top level...
> 
> Qian Zhang wrote:
>     > Should we define this next to CSIPluginContainerInfo for consistency?
>     
>     Yes, I agree.
>     
>     > Also, can we move the Service message up to the top level of the namespace without
breaking the API? Would be nice for Service to sit higher now that it's used by two messages;
but the name might be too generic for the top level...
>     
>     Did you mean the `Service` enum definition? Actually I thought it before and I agree
we may have to rename it to something CSI specific, like `CSIPluginService`. I really want
to do this refactor, but I guess that may break backward compatibility since DSS is currrently
using it.

Yea I guess we need to leave the current enum where it is. So we could either add a second
enum at the top level or use the existing one? I think I'm fine with either way, neither of
them is ideal.


- Greg


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


On July 16, 2020, 1:47 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72661/
> -----------------------------------------------------------
> 
> (Updated July 16, 2020, 1:47 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10148
>     https://issues.apache.org/jira/browse/MESOS-10148
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated `CSIPluginInfo` for supporting 3rd party CSI plugins.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 5f795f5dbebc5d79791a011a47d21859ac6cb129 
>   include/mesos/v1/mesos.proto 07d2f4012551fbb941f5b3caad6caecbafbccfb5 
> 
> 
> Diff: https://reviews.apache.org/r/72661/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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