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 proxy to set stream level downstream read timeout #507

Conversation

xushichangdesmond
Copy link
Contributor

these timeouts were already implemented and had unit tests, but were missing the API on the Session to allow configurability from ProxyHttp callbacks.

/// This is a noop for h2.
pub fn set_read_timeout(&mut self, timeout: Duration) {
match self {
Self::H1(s) => s.set_write_timeout(timeout),
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll want to invoke set_read_timeout here rather than set_write_timeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry about that; have fixed

@drcaramelsyrup
Copy link
Contributor

Linked to #506

@andrewhavck andrewhavck self-assigned this Jan 10, 2025
@gdavidsson gdavidsson added the enhancement New feature or request label Jan 10, 2025
@andrewhavck andrewhavck added the Accepted This change is accepted by us and merged to our internal repo label Jan 23, 2025
eaufavor pushed a commit that referenced this pull request Jan 31, 2025
---
Update server.rs

Includes-commit: 3bfbd28
Includes-commit: f497a85
Replicated-from: #507
eaufavor pushed a commit that referenced this pull request Jan 31, 2025
---
Update server.rs

Includes-commit: 3bfbd28
Includes-commit: f497a85
Replicated-from: #507
@eaufavor
Copy link
Member

87ae8ce cheers!

@eaufavor eaufavor closed this Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted This change is accepted by us and merged to our internal repo enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants