Skip to content

Conversation

@linyihai
Copy link
Contributor

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
Contributor 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
Contributor 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