You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2019/01/25 00:35:55 UTC

Re: Review Request 69787: Added a forwarding mode to the test CSI plugin.

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

(Updated Jan. 25, 2019, 12:35 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


Changes
-------

Addressed Benjamin's comments and rename `proxy` to `forward` for a more intuitive naming.


Summary (updated)
-----------------

Added a forwarding mode to the test CSI plugin.


Bugs: MESOS-9517
    https://issues.apache.org/jira/browse/MESOS-9517


Repository: mesos


Description (updated)
-------

If the `--forward` flag is set, the test CSI plugin would forward all gRPC
requests to the specified gRPC server URI, and return the response to
the caller. This can be used to forward all CSI calls to a mock CSI
plugin object in the test process.


Diffs (updated)
-----

  src/examples/test_csi_plugin.cpp af183037b280bab65578a4c447196a9ccf261e32 


Diff: https://reviews.apache.org/r/69787/diff/3/

Changes: https://reviews.apache.org/r/69787/diff/2-3/


Testing
-------

Manually tweaked the plugin to forward requests to itself and all tests pass.


Thanks,

Chun-Hung Hsiao


Re: Review Request 69787: Added a forwarding mode to the test CSI plugin.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69787/#review212331
-----------------------------------------------------------


Fix it, then Ship it!





src/examples/test_csi_plugin.cpp
Lines 1197-1210 (original), 1231-1244 (patched)
<https://reviews.apache.org/r/69787/#comment298093>

    Did you intend to introduce some runtime polymorphism here? Either that, or put `proxy` and `plugin` on the stack.


- Benjamin Bannier


On Jan. 25, 2019, 1:35 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69787/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2019, 1:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9517
>     https://issues.apache.org/jira/browse/MESOS-9517
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If the `--forward` flag is set, the test CSI plugin would forward all gRPC
> requests to the specified gRPC server URI, and return the response to
> the caller. This can be used to forward all CSI calls to a mock CSI
> plugin object in the test process.
> 
> 
> Diffs
> -----
> 
>   src/examples/test_csi_plugin.cpp af183037b280bab65578a4c447196a9ccf261e32 
> 
> 
> Diff: https://reviews.apache.org/r/69787/diff/3/
> 
> 
> Testing
> -------
> 
> Manually tweaked the plugin to forward requests to itself and all tests pass.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 69787: Added a forwarding mode to the test CSI plugin.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69787/
-----------------------------------------------------------

(Updated Jan. 26, 2019, 12:30 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


Changes
-------

Addressed Benjamin's comment.


Bugs: MESOS-9517
    https://issues.apache.org/jira/browse/MESOS-9517


Repository: mesos


Description
-------

If the `--forward` flag is set, the test CSI plugin would forward all gRPC
requests to the specified gRPC server URI, and return the response to
the caller. This can be used to forward all CSI calls to a mock CSI
plugin object in the test process.


Diffs (updated)
-----

  src/examples/test_csi_plugin.cpp af183037b280bab65578a4c447196a9ccf261e32 


Diff: https://reviews.apache.org/r/69787/diff/4/

Changes: https://reviews.apache.org/r/69787/diff/3-4/


Testing
-------

Manually tweaked the plugin to forward requests to itself and all tests pass.


Thanks,

Chun-Hung Hsiao