Sanjays2402 commented on code in PR #2177:
URL: https://github.com/apache/libcloud/pull/2177#discussion_r3693359496


##########
libcloud/dns/drivers/route53.py:
##########
@@ -237,13 +237,71 @@ def update_record(self, record, name=None, type=None, 
data=None, extra=None):
     def delete_record(self, record):
         try:
             r = record
-            batch = [("DELETE", r.name, r.type, r.data, r.extra)]
-            self._post_changeset(record.zone, batch)
+
+            # Multiple value records need to be handled specially - Route53
+            # only accepts a DELETE for a record set which lists every value
+            # in that set, so values for the other records need to be sent
+            # as well.
+
+            if r.extra.get("_multi_value", False) and 
r.extra.get("_other_records", []):
+                self._delete_multi_value_record(record=r)
+            else:
+                batch = [("DELETE", r.name, r.type, r.data, r.extra)]
+                self._post_changeset(record.zone, batch)

Review Comment:
   Good catch - that was a real gap. A `Record` from `create_record()` / 
`ex_create_multi_value_record()` (or one built by hand) has no `_multi_value` / 
`_other_records`, so both `delete_record()` and `update_record()` would send a 
single-value changeset and Route53 would reject it.
   
   Pushed 6e2579e. Added `_with_record_set_metadata()`, called from both 
`update_record()` and `delete_record()`: if `_multi_value` is absent from 
`record.extra` it looks the record set up in the zone and merges the metadata 
in, keeping any caller-supplied extras (ttl, priority). If the lookup fails or 
the record is not found it falls back to the old behaviour rather than raising.
   
   I used `list_records()` rather than `get_record()` for the lookup, since 
`get_record()` only asks Route53 for `maxitems=1` and then rejects the result 
when the returned member is not the one requested - for a multi value MX set 
that raises `RecordDoesNotExistError` before the metadata can be read.
   
   New test `test_delete_multi_value_record_without_record_set_metadata` builds 
a bare `Record` with no metadata and asserts the DELETE changeset contains all 
five MX values. It fails without the driver change (only `1 
ASPMX.L.GOOGLE.COM.` is emitted). `libcloud/test/dns/test_route53.py` is 25 
passed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to