You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2017/11/16 00:33:13 UTC
Re: Review Request 63385: Added utility functions for CSI responses
and volume ID and metadata.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63385/#review191126
-----------------------------------------------------------
src/csi/utils.cpp
Lines 41-46 (patched)
<https://reviews.apache.org/r/63385/#comment268736>
This comparison _looks_ wrong :)
You should note that the object is empty, so they must be equal.
src/csi/utils.cpp
Lines 62-71 (patched)
<https://reviews.apache.org/r/63385/#comment268733>
Does this version of the proto have the `access_mode` field?
https://github.com/container-storage-interface/spec/blob/2561947ce5899bcb80e5dbaecb99afdce66e91d4/csi.proto#L228
If not, consider adding a TODO?
---
Also, it might be a good idea to add a note about the Proto3 `oneof` notation which enforces the existence of either `block` or `mount`. I'm guessing most contributors reading through this code might be more familiar with Proto2 rules.
src/csi/utils.cpp
Lines 125-127 (patched)
<https://reviews.apache.org/r/63385/#comment268737>
Probably want to leave a TODO here, since `VolumeMetaData` no longer exists:
https://github.com/container-storage-interface/spec/commit/d2bdb9177e192ecee994c549f1b166fc073ff40b
- Joseph Wu
On Oct. 27, 2017, 4:59 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63385/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2017, 4:59 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added utility functions for CSI responses and volume ID and metadata.
>
>
> Diffs
> -----
>
> src/csi/utils.hpp PRE-CREATION
> src/csi/utils.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/63385/diff/1/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 63385: Added utility functions for CSI Plugin info
and volume attributes.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
> On Nov. 16, 2017, 12:33 a.m., Joseph Wu wrote:
> > src/csi/utils.cpp
> > Lines 41-46 (patched)
> > <https://reviews.apache.org/r/63385/diff/1/?file=1870949#file1870949line41>
> >
> > This comparison _looks_ wrong :)
> >
> > You should note that the object is empty, so they must be equal.
Removed since it is no longer needed.
> On Nov. 16, 2017, 12:33 a.m., Joseph Wu wrote:
> > src/csi/utils.cpp
> > Lines 62-71 (patched)
> > <https://reviews.apache.org/r/63385/diff/1/?file=1870949#file1870949line62>
> >
> > Does this version of the proto have the `access_mode` field?
> >
> > https://github.com/container-storage-interface/spec/blob/2561947ce5899bcb80e5dbaecb99afdce66e91d4/csi.proto#L228
> >
> > If not, consider adding a TODO?
> >
> > ---
> >
> > Also, it might be a good idea to add a note about the Proto3 `oneof` notation which enforces the existence of either `block` or `mount`. I'm guessing most contributors reading through this code might be more familiar with Proto2 rules.
Dropping this because this is no longer needed.
> On Nov. 16, 2017, 12:33 a.m., Joseph Wu wrote:
> > src/csi/utils.cpp
> > Lines 125-127 (patched)
> > <https://reviews.apache.org/r/63385/diff/1/?file=1870949#file1870949line125>
> >
> > Probably want to leave a TODO here, since `VolumeMetaData` no longer exists:
> > https://github.com/container-storage-interface/spec/commit/d2bdb9177e192ecee994c549f1b166fc073ff40b
Added a typedef for `google::protobuf::Map<string, string>` to `VolumeAttributes` and changed the conversion functions accordingly.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63385/#review191126
-----------------------------------------------------------
On Nov. 22, 2017, 5:21 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63385/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2017, 5:21 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added utility functions for CSI Plugin info and volume attributes.
>
>
> Diffs
> -----
>
> src/csi/utils.hpp PRE-CREATION
> src/csi/utils.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/63385/diff/2/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>