fix: make parent_id fields required compute move and insert methods#686
fix: make parent_id fields required compute move and insert methods#686vam-google merged 1 commit intogoogleapis:masterfrom
Conversation
google-cla
Bot
added
the
cla: yes
|
Thanks for your contribution! Unfortunately, we don't use GitHub pull |
vchudnov-g
left a comment
vchudnov-g
left a comment
There was a problem hiding this comment.
A couple of questions, but otherwise LGTM. Thanks for doing this!
| post: "/compute/v1/locations/global/firewallPolicies/{firewall_policy}/move" | ||
| }; | ||
| option (google.api.method_signature) = "firewall_policy"; | ||
| option (google.api.method_signature) = "firewall_policy,parent_id"; |
There was a problem hiding this comment.
Do we want this signature to be consistent with the previous one, so that aprent_id is always at the front (or always at the back)?
There was a problem hiding this comment.
It is already consistent. For the other one the trailing parameter is body parameter. Body parameter is always the last one. This method des not have body, that is why it does not have a parameter after parent_id.
| "move": { | ||
| "parameterOrder": [ | ||
| "firewallPolicy" | ||
| "firewallPolicy", |
There was a problem hiding this comment.
We should ping GCE as to the desired parameter order. What do other RPCs do with parentId?
There was a problem hiding this comment.
it seems like we only have parentID for firewallpolicies and global operations. It seems to me it would make sense to have parentId be the first parameter, if it is required?
2 participants
Note that the order in
parameterOrderproperty in Json file matters and must remain consistent. If we want to flip the orderfirewallPolicyandparentIdthis is the last chance to do so.