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

Add support for the DS record (with updated field names) #50

Closed
wants to merge 2 commits into from

Conversation

baest
Copy link

@baest baest commented Sep 18, 2023

Also updated tests to add a new DS record for a sub zone along with updates since an existing DS records is now handled Depends on octodns/octodns#1065

Also updated tests to add a new DS record for a sub zone along with updates since an existing DS records is now handled
Depends on octodns/octodns#1065
@ross
Copy link
Contributor

ross commented Sep 19, 2023

This PR will need to hold off until octodns/octodns#1065 is merged, a new octoDNS release is made, AND we're ok with hard requiring that version to use octodns-powerdns.

For now that means #49 (comment) makes sense. If I had to guess i'd say this PR can be considered for merging ~6 months or so after octodns/octodns#1065 is merged and a release with it cut.

@baest
Copy link
Author

baest commented Sep 22, 2023

I have an idea to handle this interim period with some version checking of octodns. Since if octodns/octodns#1065 is merged this will start failing.
We can undo the changes to tests/config/unit.tests.yaml and use old field names for now since that is backwards compatible. In __init__.py we can check the version of octodns and behave accordingly (I have a local patch). Problem is that we then violate the 100% test coverage since octodns can't be installed as both before and after #1065.

version_check.patch

@ross
Copy link
Contributor

ross commented Sep 24, 2023

version_check.patch

This patch got me thinking about options for how to navigate our way out of the compatibility issues outlined in octodns/octodns#1065 (comment). I came up with #51 which should do the trick.

Instead of looking at octoDNS versions it instead looks directly at the functionality it's handling, namely whether DsValue.flags is a property. If it is then it uses old names, if not new.

Closing this as #51 will be the path forward.

@ross ross closed this Sep 24, 2023
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.

2 participants