Skip to content

ensure dot in zone:id #381

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

Merged
merged 2 commits into from
May 30, 2019
Merged

Conversation

splashx
Copy link
Contributor

@splashx splashx commented Mar 19, 2019

Closes #380

@CapsuleCD
Copy link
Contributor

Hi.

I'm an automated pull request bot named CapsuleCD. I handle testing, versioning and package releases for this project.

  • If you're the owner of this repo, you can click the button below to kick off the CapsuleCD build pipeline and create an automated release.'
  • If not, don't worry, someone will be by shortly to check on your pull request.

CapsuleCD


If you're interested in learning more about CapsuleCD, or adding it to your project, you can check it out here

@splashx
Copy link
Contributor Author

splashx commented Mar 19, 2019

I want to point out this is passing, the problem is with pylint+tox causing all sort of problems :(
Increasing the version of pylint will create a bunch of false positives for R0801 (see pylint-dev/pylint#1055 - which actually didnt solve the problem)

@adferrand
Copy link
Collaborator

Hello @splashx, I fixed the pylint errors, you should be able to fix your PR tests with a merge from master.

@splashx splashx force-pushed the add-support-for-dot-notation branch from e004ab5 to 4add053 Compare April 16, 2019 15:26
@splashx splashx force-pushed the add-support-for-dot-notation branch from 4add053 to 32ba9bd Compare May 20, 2019 14:58
@splashx
Copy link
Contributor Author

splashx commented May 20, 2019

@adferrand done. Please review the AppVeyor CI part as I think it's not related to these changes.

@AnalogJ
Copy link
Owner

AnalogJ commented May 21, 2019

Hey @splashx Thanks for the PR!
Would the _fqdn_name helper do the same thing that you're trying to do?

https://github.com/AnalogJ/lexicon/blob/master/lexicon/providers/base.py#L153

@splashx
Copy link
Contributor Author

splashx commented May 21, 2019

@AnalogJ not really. PowerDNS expects the zone:id (not the fqdn) in a dot-format.
I also didn’t want to plumb this at a higher level and change self.domain because that breaks some logics. So I think this is the appropriate place for this.
Currently those using lexicon and dealing with PowerDNS must write their own “ensure dot” function and pass that for every zone-handling action. It’s tedious.

@AnalogJ AnalogJ merged commit 0ac45bb into AnalogJ:master May 30, 2019
@AnalogJ
Copy link
Owner

AnalogJ commented May 30, 2019

LGTM. Thanks for the PR @splashx 🎉

chrisbraucker pushed a commit to chrisbraucker/lexicon that referenced this pull request Jan 1, 2020
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.

PowerDNS dot notation
4 participants