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

Make ExchangeWithConnContext public #1459

Merged
merged 1 commit into from Jun 19, 2023

Conversation

andrew-d
Copy link
Contributor

If we want to use a custom mechanism of obtaining a Conn that doesn't match the net.Dialer type, but retain the timeout behaviour from ExchangeContext, there was previously no way to accomplish this.

This PR makes the underlying ExchangeWithConnContext function public, which allows this behaviour.

Now that #1454 is merged, there is no longer any interaction between the context provided and the singleflight behaviour, so I removed the comment from ExchangeWithConn.

If we want to use a custom mechanism of obtaining a Conn that doesn't
match the net.Dialer type, but retain the timeout behaviour from
ExchangeContext, there was previously no way to accomplish this.

This PR makes the underlying ExchangeWithConnContext function public,
which allows this behaviour.

Now that miekg#1454 is merged, there is no longer any interaction between the
context provided and the singleflight behaviour, so I removed the
comment from ExchangeWithConn.

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
@andrew-d
Copy link
Contributor Author

@miekg @tmthrgd gentle ping on this; anything I can do to help move this forward? Happy to e.g. write additional tests.

@miekg
Copy link
Owner

miekg commented Jun 16, 2023

If we want to use a custom mechanism of obtaining a Conn that doesn't match the net.Dialer type, but retain the timeout behaviour from ExchangeContext, there was previously no way to accomplish this.

I don't understand... you mean doesn't match the net.Conn interface or some such? As exchangeWithConnContext has the identical paramaters (apart from the context) I don't see how this would help

@andrew-d
Copy link
Contributor Author

If we want to use a custom mechanism of obtaining a Conn that doesn't match the net.Dialer type, but retain the timeout behaviour from ExchangeContext, there was previously no way to accomplish this.

I don't understand... you mean doesn't match the net.Conn interface or some such? As exchangeWithConnContext has the identical paramaters (apart from the context) I don't see how this would help

Concretely: if I need to do something to obtain a *dns.Conn other than using a *net.Dialer (through Client.Dialer), there's no way to both do that and add a timeout to the request. I can do something like:

netConn, err := myDialFunc(/* args */)
// check err
conn := &dns.Conn{
	Conn: netConn,
}

var c dns.Client
resp, _, err := c.ExchangeWithConn(msg, conn)

However, there's no way to add a timeout to the ExchangeWithConn function. ExchangeContext doesn't let me provide a *dns.Conn, and the Dialer field of Client is of concrete type *net.Dialer so I can't override it with my custom dial logic.

Exposing ExchangeWithConnContext would allow me to pass a context.Context with an appropriate deadline and thus add a timeout to the query.

@miekg
Copy link
Owner

miekg commented Jun 18, 2023 via email

@miekg miekg merged commit 8b8cf14 into miekg:master Jun 19, 2023
@andrew-d andrew-d deleted the andrew/exchange-conn-context branch June 19, 2023 14:06
@andrew-d
Copy link
Contributor Author

Thank you!

ydnar added a commit to domainr/dnsr that referenced this pull request Jun 21, 2023
ydnar added a commit to domainr/dnsr that referenced this pull request Jun 22, 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.

None yet

2 participants