You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2017/08/08 08:21:03 UTC

Re: Review Request 58021: Added storage-related offer operations.

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

(Updated Aug. 8, 2017, 10:21 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
-------

Added storage-related offer operations.


Diffs (updated)
-----

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
  src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
  src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 


Diff: https://reviews.apache.org/r/58021/diff/5/

Changes: https://reviews.apache.org/r/58021/diff/4-5/


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 58021: Added storage-related offer operations.

Posted by Jie Yu <yu...@gmail.com>.

> On Sept. 6, 2017, 8:55 p.m., James DeFelice wrote:
> > include/mesos/mesos.proto
> > Lines 1835 (patched)
> > <https://reviews.apache.org/r/58021/diff/7/?file=1808481#file1808481line1835>
> >
> >     I'd love to see documentation for these new operations to distinguish them from the CREATE and DESTROY operations

Yeah, this is WIP. Will definitely follow up with docs for that. We'll likely introduce alias for existing CREATE/DESTROY


- Jie


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


On Aug. 31, 2017, 11:33 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2017, 11:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
>     https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
>   include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 
>   src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
>   src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
>   src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
>   src/master/master.cpp 4fa05fa309e4302d18cb9557a4730bd5f8dd29e6 
>   src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/7/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 58021: Added storage-related offer operations.

Posted by James DeFelice <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58021/#review184731
-----------------------------------------------------------




include/mesos/mesos.proto
Lines 1835 (patched)
<https://reviews.apache.org/r/58021/#comment260925>

    I'd love to see documentation for these new operations to distinguish them from the CREATE and DESTROY operations


- James DeFelice


On Aug. 31, 2017, 11:33 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2017, 11:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
>     https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
>   include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 
>   src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
>   src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
>   src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
>   src/master/master.cpp 4fa05fa309e4302d18cb9557a4730bd5f8dd29e6 
>   src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/7/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 58021: Added storage-related offer operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58021/
-----------------------------------------------------------

(Updated Aug. 31, 2017, 1:33 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
-------

Addressed issues.


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


Repository: mesos


Description
-------

Added storage-related offer operations.


Diffs (updated)
-----

  include/mesos/mesos.proto eede0827fc0da5f2fa3cfb432fde29b95a8c644c 
  include/mesos/v1/mesos.proto 8c6246ee824f28d93dc5b57d25939d1ba76c986b 
  src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
  src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
  src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
  src/master/master.cpp 4fa05fa309e4302d18cb9557a4730bd5f8dd29e6 
  src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 


Diff: https://reviews.apache.org/r/58021/diff/7/

Changes: https://reviews.apache.org/r/58021/diff/6-7/


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 58021: Added storage-related offer operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

> On Aug. 31, 2017, 4:14 a.m., Jie Yu wrote:
> > src/common/protobuf_utils.cpp
> > Lines 532-538 (original), 574-584 (patched)
> > <https://reviews.apache.org/r/58021/diff/6/?file=1806578#file1806578line575>
> >
> >     Can you change this too? like what we did for inject?

Oh, of course. Not sure how I could miss that.


- Jan


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


On Aug. 29, 2017, 3:23 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2017, 3:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
>     https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
>   include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
>   src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
>   src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
>   src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
>   src/master/master.cpp 183f53070848853b50bfb02bcd6b458ac0b2f685 
>   src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 58021: Added storage-related offer operations.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58021/#review184228
-----------------------------------------------------------


Fix it, then Ship it!





src/common/protobuf_utils.cpp
Lines 532-538 (original), 574-584 (patched)
<https://reviews.apache.org/r/58021/#comment260319>

    Can you change this too? like what we did for inject?


- Jie Yu


On Aug. 29, 2017, 1:23 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2017, 1:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
>     https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
>   include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
>   src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
>   src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
>   src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
>   src/master/master.cpp 183f53070848853b50bfb02bcd6b458ac0b2f685 
>   src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 58021: Added storage-related offer operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58021/
-----------------------------------------------------------

(Updated Aug. 29, 2017, 3:23 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
-------

Addressed issues.


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


Repository: mesos


Description
-------

Added storage-related offer operations.


Diffs (updated)
-----

  include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
  include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
  src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
  src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
  src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
  src/master/master.cpp 183f53070848853b50bfb02bcd6b458ac0b2f685 
  src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 


Diff: https://reviews.apache.org/r/58021/diff/6/

Changes: https://reviews.apache.org/r/58021/diff/5-6/


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 58021: Added storage-related offer operations.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58021/#review183971
-----------------------------------------------------------




src/common/protobuf_utils.cpp
Line 442 (original), 442 (patched)
<https://reviews.apache.org/r/58021/#comment260009>

    I'd see if we can avoid changing `inject` to different methods based on the type. See if something like the following will work or not:
    
    ```
    struct Injector
    {
      void operator()(
          Resource& resource,
          const Reosurce::AllocationInfo& allocationInfo)
      { ... }
      
      void operator()(
          RepeatedPtrField<Resource>* resources,
          const Reosurce::AllocationInfo& allocationInfo)
      { ... }
    } inject;
    ```


- Jie Yu


On Aug. 8, 2017, 8:21 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2017, 8:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
>     https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
>   src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
>   src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>