-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
"projects/{project}/zones/{zone}/disks/{name}/setLabels" | ||
]).format(**module.params), | ||
{ | ||
u'labels': module.params.get('labels') |
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.
You'll also need to send labelFingerprint, here and elsewhere.
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.
Do I need to? I currently do not support beta APIs on Ansible, but we list all of the labelFingerprints as beta only.
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.
Huh! Well, you do need to, but that probably represents an error in api.yaml.
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.
Disk labels are GA!
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 doesn't seem to have been fixed.
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.
Do we want to fix that in this PR? I didn't know if that would clutter it up too much (we'd have changes in P/C/A/T from doing that)
Should've specified that earlier.
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.
Probably best to do it - otherwise you're submitting code that doesn't work. :(
b86f025
to
f44a027
Compare
@@ -427,6 +427,8 @@ def main(): | |||
if fetch: | |||
if state == 'present': | |||
if is_different(module, fetch): | |||
update_fields(module, resource_to_request(module), | |||
response_to_hash(module, fetch)) | |||
fetch = update(module, self_link(module), kind) |
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.
Same deal here as the others- it should be either a partial or a full, not both
f44a027
to
d303922
Compare
u'labels': module.params.get('labels') | ||
} | ||
) | ||
if response.get('sizeGb') != request.get('sizeGb'): |
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.
Someday, you're going to want to inject a "not quite just literal comparisons only" bit here - a canonicalizer, we used to call them in Maps. Don't have to do it today.
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.
Good call! I'm going to make a issue for this.
@@ -236,6 +236,12 @@ | |||
- Labels to apply to this VpnTunnel. | |||
returned: success | |||
type: dict | |||
label_fingerprint: |
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.
You might find this upsetting, but unfortunately networking resources only accept labels via SetLabels calls - they do not work on create.
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.
You'll probably want to figure out the networking resources create vs update issue - if you don't want to figure it out today, let me know and I imagine some functionality is better than no functionality.
I updated this with VpnTunnel. Not sure what you consider a networking resource. |
d303922
to
1298b1a
Compare
sizegb_update(module, request, response) | ||
|
||
|
||
def labelfingerprint_update(module, request, response): |
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.
It would help readability for this to be label_fingerprint_update
.
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.
Good call. Fixed! Running through the magician now.
1298b1a
to
d5cfd99
Compare
* add locks * rename the module * add test * add test * address comments * add quote * can list child scope lock * minor docs tweaks * Add files via upload (#62) * change '\r\n' to '\n' (#63) * Small changes, just to trigger CI verify. * trigger CI verify * remove 's' * Update according by comments * change small for trigger CI check
/cc @rambleraptor