-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proto: bump to 1.6.x 2f6f381 #354
Conversation
82ed45f
to
e3a9d93
Compare
e3a9d93
to
ec04a93
Compare
@@ -239,7 +239,8 @@ message Response { | |||
repeated OperationStatus operation_statuses = 1 [(gogoproto.nullable) = false]; | |||
} | |||
|
|||
optional ReconcileOperations reconcile_operations = 1; | |||
optional Type type = 1 [(gogoproto.nullable) = false]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this tag change is unfortunate but aligns perfectly w/ upstream changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked with Gaston and confirmed that this was not used anywhere when introduced, so it should be a "safe" change.
api/v1/lib/master/calls/calls.go
Outdated
@@ -221,6 +228,32 @@ func DestroyVolumes(a mesos.AgentID, v ...mesos.Resource) *master.Call { | |||
} | |||
} | |||
|
|||
// GrowLocalVolume grows a persistent volume on an agent's ROOT or PATH disks. The request is forwarded asynchronously | |||
// to the Mesos agent where the persistent volume is located. | |||
func GrowLocalVolume(a mesos.AgentID, volume mesos.Resource, addition mesos.Resource) *master.Call { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it's more idiomatic to say volume, addition mesos.Resource
(since they've of the same type)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also: mesos.AgentID is optional in the protos, so it should be a pointer here
api/v1/lib/master/calls/calls.go
Outdated
|
||
// ShrinkLocalVolume shrinks a persistent volume on an agent's ROOT or PATH disks. The request is forwarded | ||
// asynchronously to the Mesos agent where the persistent volume is located. | ||
func ShrinkLocalVolume(a mesos.AgentID, volume mesos.Resource, subtract mesos.Value_Scalar) *master.Call { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mesos.AgentID is optional in protos, should be a pointer parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make it a pointer. Then should I just rename it to GrowVolume
/ShrinkVolume
? Currently it supports only local volumes but me may extend it to support external volumes in the future.
Thanks, left some feedback. Surprised that schedulerpb_test.go didn't have any changes |
I'm fine w/ GrowVolume/ShrinkVolume
…On Thu, Jul 19, 2018 at 1:56 PM Chun-Hung Hsiao ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In api/v1/lib/master/calls/calls.go
<#354 (comment)>:
> +// GrowLocalVolume grows a persistent volume on an agent's ROOT or PATH disks. The request is forwarded asynchronously
+// to the Mesos agent where the persistent volume is located.
+func GrowLocalVolume(a mesos.AgentID, volume mesos.Resource, addition mesos.Resource) *master.Call {
+ return &master.Call{
+ Type: master.Call_GROW_VOLUME,
+ GrowVolume: &master.Call_GrowVolume{
+ AgentID: &a,
+ Volume: volume,
+ Addition: addition,
+ },
+ }
+}
+
+// ShrinkLocalVolume shrinks a persistent volume on an agent's ROOT or PATH disks. The request is forwarded
+// asynchronously to the Mesos agent where the persistent volume is located.
+func ShrinkLocalVolume(a mesos.AgentID, volume mesos.Resource, subtract mesos.Value_Scalar) *master.Call {
I'll make it a pointer. Then should I just rename it to GrowVolume/
ShrinkVolume? Currently it supports only local volumes but me may extend
it to support external volumes in the future.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#354 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACPVLNWkO781DWvBzb6AXsMFyIrFHC3Xks5uIMhSgaJpZM4VVail>
.
|
ca7d0a4
to
c11ae24
Compare
@jdef Done. PTAL. |
Updated Mesos protos to 1.6.x, which adds a new master call for GET_OPERATIONS, and new agent calls for GET_OPERATIONS, GROW_VOLUME and SHRINK_VOLUME. For the latter two, only agent-local volumes are supported.