----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64868/#review194600 -----------------------------------------------------------
docs/csi.md Lines 137 (patched) <https://reviews.apache.org/r/64868/#comment273452> > Any disk resource ... inherits the same profile as the storage pool. Why, and who enforces this? Is this because a single logical pool may have multiple, virtual representations (one per profile)? If so it might be worth differentiating between a logical pool and a virtual one. docs/csi.md Lines 151 (patched) <https://reviews.apache.org/r/64868/#comment273453> Can we make it clear that we're talking about a hypothetical EBS plugin? AFAIK the external resource provider stuff isn't in place yet (and may contain unexpected monsters) docs/csi.md Lines 156 (patched) <https://reviews.apache.org/r/64868/#comment273454> s/is likely to/may/ docs/csi.md Lines 166 (patched) <https://reviews.apache.org/r/64868/#comment273456> s/about// docs/csi.md Lines 194 (patched) <https://reviews.apache.org/r/64868/#comment273455> it may be worth elaborating on the difference between CREATE and CREATE_VOLUME. it's a very confusing naming scheme. docs/csi.md Lines 242 (patched) <https://reviews.apache.org/r/64868/#comment273457> suggest: Currently, Mesos infers the result based on the presence an assigned profile in the disk resource. docs/csi.md Lines 290 (patched) <https://reviews.apache.org/r/64868/#comment273458> s/that is/that are/ docs/csi.md Lines 298 (patched) <https://reviews.apache.org/r/64868/#comment273459> this feels like a separate feature. should it be documented elsewhere? docs/csi.md Lines 369 (patched) <https://reviews.apache.org/r/64868/#comment273460> it's worth noting that because this is based on the CSI specification, there's an implicit dependency between backward compatibility of the module interface and the CSI specification version. In other words, there's already a compatiblity scheme for Mesos modules that was crafted prior to CSI. CSI doesn't provide a backward compatibility promise. As CSI evolves, this module interface may implicitly evolve in a way that deviates from the compatibility promise described here: https://github.com/apache/mesos/blob/master/docs/modules.md#module-versioning-and-backwards-compatibility .. or, maybe I misunderstand our guarantees re: module backwards compat. docs/csi.md Lines 385 (patched) <https://reviews.apache.org/r/64868/#comment273461> I'd like to see it called out clearly in the text (vs. being embedded in a link) the specific version of CSI that Mesos currently supports. Maybe this is already called out and I missed it? docs/csi.md Lines 415 (patched) <https://reviews.apache.org/r/64868/#comment273462> this is a protobuf3 type, for which there are specific rules re: JSON-ification: https://developers.google.com/protocol-buffers/docs/proto3#json In particular: > Message field names are mapped to lowerCamelCase and become JSON object keys. null is accepted and treated as the default value of the corresponding field type. This difference is probably important to note. The example provided continues to use snake_case for protobuf3 json fields (e.g. fs_type vs fsType). - James DeFelice On Dec. 29, 2017, 4:53 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64868/ > ----------------------------------------------------------- > > (Updated Dec. 29, 2017, 4:53 a.m.) > > > Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, > Greg Mann, Joseph Wu, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > Added initial doc about CSI support in Mesos. > > > Diffs > ----- > > docs/csi.md PRE-CREATION > > > Diff: https://reviews.apache.org/r/64868/diff/2/ > > > Testing > ------- > > The rendering can be checked here: > https://github.com/jieyu/mesos/blob/csi_doc/docs/csi.md > > > Thanks, > > Jie Yu > >