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

feat(redirect): policies from tower-http can be used as custom policy #2613

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

linyihai
Copy link

What this PR want

Address #2576

How to implemente

Add a convert function, which can convert tower-http policy to rewqest redirect policy.

A base example could be

  use tower_http::follow_redirect::policy::{FilterCredentials, Limited};
  let _policy = Policy::custom(tower_policy(FilterCredentials::new()));

@linyihai
Copy link
Author

Add a convert function, which can convert tower-http policy to rewqest redirect policy.

Hello, could anyone give me some feebacks? IMO, this solution is concise, and don't bring big change.

@seanmonstar
Copy link
Owner

Hey there! Thanks for working on this! I do think we could eventually consider how to make it easy to use policies from tower-http. But at the same time, publicly depending on the Policy trait means we can't upgrade tower-http freely, because that would then be a breaking change in reqwest.

The goal of the issue is kind of the opposite way around from this PR: make the internals of reqwest (so, the redirect loop inside the PendingRequest future) to use tower_http::redirect. So as to not be a breaking change to users, it'd still look like setting redirects(Policy::limit(5)), and inside we'd turn that into a tower_http::redirect::Policy. Does that make sense?

@linyihai
Copy link
Author

Thanks your reply!

But at the same time, publicly depending on the Policy trait means we can't upgrade tower-http freely, because that would then be a breaking change in reqwest.

Oh, that's a big problem.

The goal of the issue is kind of the opposite way around from this PR: make the internals of reqwest (so, the redirect loop inside the PendingRequest future) to use tower_http::redirect. So as to not be a breaking change to users, it'd still look like setting redirects(Policy::limit(5)), and inside we'd turn that into a tower_http::redirect::Policy. Does that make sense?

Ok, I'd try this approach.

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