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

Allow built-in IPv{4,6}Address objects when constructing A/AAAA rdata #1183

Merged
merged 2 commits into from
Mar 17, 2025

Conversation

balaziks
Copy link
Contributor

@balaziks balaziks commented Mar 5, 2025

This pull request adds support of Python's built-in ipaddress.IPv{4,6}Address types for constructing Rdata of type A and AAAA. This also adds support in other places dns.rdata.Rdata._as_ipv{4,6}_address class method is used.

In testing this I also encountered a cryptic error message from dns.rdata.from_text which I improved to be more specific.

Copy link
Collaborator

@bwelling bwelling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good other than a minor issue.

Also applies elsewhere where dns.rdata.Rdata._as_ipv{4,6}_address is
used.
This now returns a better error message. Previously:

`SyntaxError: 'foo' object has no attribute 'get'`

Now:

`ValueError: tok must be a string or a Tokenizer`
@balaziks balaziks force-pushed the rdtypes_a_aaaa_builtin_ipaddress branch from 3f3f850 to b49aec8 Compare March 10, 2025 18:40
@balaziks balaziks requested a review from bwelling March 10, 2025 18:41
Copy link
Collaborator

@bwelling bwelling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@rthalley rthalley merged commit fee25b8 into rthalley:main Mar 17, 2025
8 checks passed
@balaziks balaziks deleted the rdtypes_a_aaaa_builtin_ipaddress branch March 18, 2025 10:52
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.

3 participants