Add PTR record support #164
Conversation
There was a problem hiding this comment.
Looking good - a couple of questions in the review
Also wondering about GoogleCloudDNS and OracleCloudDNS providers. What's our stance on adding functionality, but not adding in all providers? Does it produce a reasonable error at least if we try to use there?
| assert_equal( | ||
| '4e0ebbeac8d2e4e73af888b20e2243e5a2a08bad6476c832c985e54b21eff4a3', | ||
| matching_records.first.fingerprint | ||
| ) |
There was a problem hiding this comment.
seems unrelated... a leak of something into this PR?
There was a problem hiding this comment.
Yes, it's unrelated, but deliberately so. I thought I could sneak in some changes here to strengthen tests from the SSHFP pull request as I noticed they weren't really asserting that any records had been persisted.
I'm willing to just back this change out. The response is stored on cassette anyway so it's really only verifying a change took place on the remote server the very first time it's run.
There was a problem hiding this comment.
I'm fine with the occasional drive-by :-)
mostly making sure I wasn't missing something, and can see the commit now
for future, maybe the commit comment could be more description for cases like this, or a note added to the PR description to call out the out-of-scope change -- ie. the comment "Assorted test cleanup" didn't clearly tell me what it was doing, and if I were looking at this and doing some archaeology in future it might be confusing
|
With respect to adding support in the Providers must explicitly declare what record types they support. I have characterized the behaviour in some new tests. Otherwise the Zone will raise a "PTR is not a supported record type" validation error. |
|
Just pushed up some additional validations for PTR FQDNs:
|
Add support for reading/writing PTR records to both NS1 and DNSimple. Such records can already be read using the DynECT provider.