Skip to content
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

Optional support for multi-value PTRs #9

Merged
merged 1 commit into from
Oct 17, 2022
Merged

Conversation

mochipon
Copy link
Contributor

@mochipon mochipon commented Oct 17, 2022

Multi-value PTR records is indeed supported by octoDNS.
The problem is if unsupported providers (e.g., PowerDNS, Google Cloud DNS) are used. In a backward compatible way, octoDNS will leave only one value.

https://github.com/octodns/octodns/blob/6890e3307c7246720820e4dc8f18edc2c8bcf164/octodns/provider/base.py#L91-L102

However, we cannot choose which to keep. The current implementation seems to keep the first value sorted by name, but this may not be what the user actually wants.

For instance, I might type v1001.rtr.example.com,rtr.example.com in the description field. In this case, rtr.example.com will always be selected, but is this the desired result for the operator?

@mochipon mochipon force-pushed the fix_multipleptr_order branch from 7585099 to ba4d2f7 Compare October 17, 2022 10:45
@sukiyaki sukiyaki deleted a comment from codecov bot Oct 17, 2022
@sukiyaki sukiyaki deleted a comment from codeclimate bot Oct 17, 2022
@codecov
Copy link

codecov bot commented Oct 17, 2022

Codecov Report

Merging #9 (ba4d2f7) into main (167f0a9) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main        #9   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           46        46           
=========================================
  Hits            46        46           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@codeclimate
Copy link

codeclimate bot commented Oct 17, 2022

Code Climate has analyzed commit ba4d2f7 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 100.0% (0.0% change).

View more on Code Climate.

@mochipon mochipon merged commit 8cdfe22 into main Oct 17, 2022
@mochipon mochipon deleted the fix_multipleptr_order branch October 17, 2022 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant