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 2018/12/07 04:31:52 UTC
Re: Review Request 69037: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69037/
-----------------------------------------------------------
(Updated Dec. 7, 2018, 4:31 a.m.)
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Changes
-------
Updated the descriptions and implemented resource comparison and printing logic.
Summary (updated)
-----------------
Added an optional `vendor` field to `Resource.DiskInfo.Source`.
Bugs: MESOS-9321
https://issues.apache.org/jira/browse/MESOS-9321
Repository: mesos
Description (updated)
-------
The new `vendor` field together with the `id` field provide the means of
uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
there is a change in the agent ID).
Diffs (updated)
-----
include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5
include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5
src/common/resources.cpp 1585e30ae9b8ba14b67deed33d67fa2d88576caa
src/v1/resources.cpp 0248f1da650b4973d09eaab3cfbe5a113b5a447a
Diff: https://reviews.apache.org/r/69037/diff/2/
Changes: https://reviews.apache.org/r/69037/diff/1-2/
Testing (updated)
-------
make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 69037: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69037/
-----------------------------------------------------------
(Updated Dec. 13, 2018, 10:14 p.m.)
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Changes
-------
Addressed Benjamin's comment.
Bugs: MESOS-9321
https://issues.apache.org/jira/browse/MESOS-9321
Repository: mesos
Description
-------
The new `vendor` field together with the `id` field provide the means of
uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
there is a change in the agent ID).
Diffs (updated)
-----
include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5
include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5
src/common/resources.cpp 1585e30ae9b8ba14b67deed33d67fa2d88576caa
src/v1/resources.cpp 0248f1da650b4973d09eaab3cfbe5a113b5a447a
Diff: https://reviews.apache.org/r/69037/diff/3/
Changes: https://reviews.apache.org/r/69037/diff/2-3/
Testing
-------
Testing done later in chain.
Thanks,
Chun-Hung Hsiao
Re: Review Request 69037: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On Dec. 13, 2018, 12:14 p.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto
> > Lines 1467-1468 (patched)
> > <https://reviews.apache.org/r/69037/diff/2/?file=2112533#file2112533line1467>
> >
> > I wonder if we should even document this implementation (?) aspect. It doesn't seem to add value.
> >
> > Also in v1.
It depends on if we want the framework to be able to select offers based on this field. If you think this does not jusdify the explicitness of this behavior I'll remove it.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69037/#review211281
-----------------------------------------------------------
On Dec. 7, 2018, 4:31 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69037/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9321
> https://issues.apache.org/jira/browse/MESOS-9321
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new `vendor` field together with the `id` field provide the means of
> uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
> there is a change in the agent ID).
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5
> include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5
> src/common/resources.cpp 1585e30ae9b8ba14b67deed33d67fa2d88576caa
> src/v1/resources.cpp 0248f1da650b4973d09eaab3cfbe5a113b5a447a
>
>
> Diff: https://reviews.apache.org/r/69037/diff/2/
>
>
> Testing
> -------
>
> Testing done later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69037: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
Posted by James DeFelice <ja...@gmail.com>.
> On Dec. 13, 2018, 12:14 p.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto
> > Lines 1467-1468 (patched)
> > <https://reviews.apache.org/r/69037/diff/2/?file=2112533#file2112533line1467>
> >
> > I wonder if we should even document this implementation (?) aspect. It doesn't seem to add value.
> >
> > Also in v1.
>
> Chun-Hung Hsiao wrote:
> It depends on if we want the framework to be able to select offers based on this field. If you think this does not jusdify the explicitness of this behavior I'll remove it.
>
> Chun-Hung Hsiao wrote:
> Also this would emphesize the importance of making (type, name) unique for a CSI plugin.
If we want frameworks to understand the structure of this field, then it should be a proper message type instead of a string. Either this field is opaque to frameworks (and we rip out the commentary about how its constructed) or it's exposed as a first-class message type.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69037/#review211281
-----------------------------------------------------------
On Dec. 7, 2018, 4:31 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69037/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9321
> https://issues.apache.org/jira/browse/MESOS-9321
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new `vendor` field together with the `id` field provide the means of
> uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
> there is a change in the agent ID).
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5
> include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5
> src/common/resources.cpp 1585e30ae9b8ba14b67deed33d67fa2d88576caa
> src/v1/resources.cpp 0248f1da650b4973d09eaab3cfbe5a113b5a447a
>
>
> Diff: https://reviews.apache.org/r/69037/diff/2/
>
>
> Testing
> -------
>
> Testing done later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69037: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On Dec. 13, 2018, 12:14 p.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto
> > Lines 1467-1468 (patched)
> > <https://reviews.apache.org/r/69037/diff/2/?file=2112533#file2112533line1467>
> >
> > I wonder if we should even document this implementation (?) aspect. It doesn't seem to add value.
> >
> > Also in v1.
>
> Chun-Hung Hsiao wrote:
> It depends on if we want the framework to be able to select offers based on this field. If you think this does not jusdify the explicitness of this behavior I'll remove it.
Also this would emphesize the importance of making (type, name) unique for a CSI plugin.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69037/#review211281
-----------------------------------------------------------
On Dec. 7, 2018, 4:31 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69037/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9321
> https://issues.apache.org/jira/browse/MESOS-9321
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new `vendor` field together with the `id` field provide the means of
> uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
> there is a change in the agent ID).
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5
> include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5
> src/common/resources.cpp 1585e30ae9b8ba14b67deed33d67fa2d88576caa
> src/v1/resources.cpp 0248f1da650b4973d09eaab3cfbe5a113b5a447a
>
>
> Diff: https://reviews.apache.org/r/69037/diff/2/
>
>
> Testing
> -------
>
> Testing done later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69037: Added an optional `vendor` field to
`Resource.DiskInfo.Source`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69037/#review211281
-----------------------------------------------------------
Fix it, then Ship it!
include/mesos/mesos.proto
Lines 1467-1468 (patched)
<https://reviews.apache.org/r/69037/#comment296254>
I wonder if we should even document this implementation (?) aspect. It doesn't seem to add value.
Also in v1.
- Benjamin Bannier
On Dec. 7, 2018, 5:31 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69037/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2018, 5:31 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9321
> https://issues.apache.org/jira/browse/MESOS-9321
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The new `vendor` field together with the `id` field provide the means of
> uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
> there is a change in the agent ID).
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5
> include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5
> src/common/resources.cpp 1585e30ae9b8ba14b67deed33d67fa2d88576caa
> src/v1/resources.cpp 0248f1da650b4973d09eaab3cfbe5a113b5a447a
>
>
> Diff: https://reviews.apache.org/r/69037/diff/2/
>
>
> Testing
> -------
>
> Testing done later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>