Skip to content

Discard crate-specific DNS resolver traits#304

Merged
wez merged 7 commits into
KumoCorp:mainfrom
djc:single-resolver
Oct 17, 2024
Merged

Discard crate-specific DNS resolver traits#304
wez merged 7 commits into
KumoCorp:mainfrom
djc:single-resolver

Conversation

@djc

@djc djc commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

r? @wez

@djc
djc force-pushed the single-resolver branch 2 times, most recently from d6433e9 to 8259962 Compare October 16, 2024 12:26
@djc
djc force-pushed the single-resolver branch 2 times, most recently from 23b8d39 to 33bc303 Compare October 16, 2024 13:12

@wez wez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this looks good, but I lost internet for a a couple of hours part way through and went on a side quest to troubleshoot why my backup connection stopped working, and I think I was in the middle of checking something to make a suggestion just before that happened, but have forgotten what it was!
I'll come back and re-review later!

Comment thread crates/dkim/src/errors.rs Outdated
}

#[derive(Default)]
pub struct TestResolver {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thinking ahead for the higher level integration test use case, where it is desirable to shim in the zone/txt records from the kumod configuration, one way to achieve that is to have a version of TestResolver that includes an Option<Box<dyn Resolver>> that is consulted as a fallback when no records are found.

I don't thinks that changes what you are doing here in this PR, I'm just "thinking out loud"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't yet understand the integration test setup well enough to speculate on this, but obviously there's a bunch of stuff we can do inside this. So far I think the deduplication of all the test resolvers and the resolver adapater [sic] have worked out pretty well...

@djc
djc force-pushed the single-resolver branch from 33bc303 to 2f3fec6 Compare October 16, 2024 20:30
@wez
wez merged commit 7c9622b into KumoCorp:main Oct 17, 2024
@wez

wez commented Oct 17, 2024

Copy link
Copy Markdown
Collaborator

Thanks!

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