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 a Resolver interface #82

Closed
jsha opened this issue Jun 22, 2020 · 4 comments · Fixed by #148
Closed

Add a Resolver interface #82

jsha opened this issue Jun 22, 2020 · 4 comments · Fixed by #148

Comments

@jsha
Copy link
Collaborator

jsha commented Jun 22, 2020

Right now ureq will always use to_socket_addrs to lookup hostnames. For testing, it would be useful to force all lookups to resolve to localhost and a test server running there. This is particularly useful when a variety of hostnames are needed, or when testing large transfers that would be expensive to send over a real network.

We should add an internal Resolver interface that uses to_socket_addrs by default, but can be overridden during testing to provide mocked-out results.

@lolgesten
Copy link
Contributor

lolgesten commented Jun 22, 2020

@jsha in hreq i got this https://docs.rs/hreq/0.3.0/hreq/trait.RequestBuilderExt.html#tymethod.with_override

Would something like that work here too? it effectively makes it so the entry URI is masked by the override host/port/tls setting. That also works on redirects, so if you follow a redirect chain:

URI1 -> URI2 -> URI1

also when you get redirected back to URI1, the override kicks in.

@jsha
Copy link
Collaborator Author

jsha commented Jun 23, 2020

Yep, the with_override method looks like a great solution! My inclination would be to enforce that it's test-only via #[cfg(test)] until someone comes along with a use case that strongly motivates making it public.

@lolgesten
Copy link
Contributor

Sure! Let's gate.

@rawler
Copy link
Contributor

rawler commented Sep 15, 2020

I have a use-case for this in a production-environment, where I'd like to ensure the "spread" of requests are even between hosts. (Basically round-robin). I'll whip up a PR and let's see what you think.

@jsha jsha closed this as completed in #148 Sep 26, 2020
jsha pushed a commit that referenced this issue Sep 26, 2020
This defines a new trait `Resolver`, which turns an address into a
Vec<SocketAddr>. It also provides an implementation of Resolver for
`Fn(&str)` so it's easy to define simple resolvers with a closure.


Fixes #82

Co-authored-by: Ulrik <ulrikm@spotify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants