Skip to content

Drop RedirectLoop exception - #819

Merged
lovelydinosaur merged 3 commits into
encode:masterfrom
oczkers:drop_redirectloop
Feb 24, 2020
Merged

lovelydinosaur merged 3 commits into
encode:masterfrom
oczkers:drop_redirectloop

Conversation

@oczkers

@oczkers oczkers commented Feb 20, 2020

Copy link
Copy Markdown
Contributor

Simply drop RedirectLoop exception and instead rely on max number of redirects (and retries on future) like requests, urllib3 and many others do.
More info in previous closed pr #801

async def test_redirect_loop():
client = AsyncClient(dispatch=MockDispatch())
with pytest.raises(RedirectLoop):
await client.get("https://example.org/redirect_loop")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess we could keep this test in, and just ensure that TooManyRedirects is raised?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is already test for TooManyRedirects with 21 redirects so i thought it's gonna be enough but sure more is better.

@lovelydinosaur lovelydinosaur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me yup!

There's one test case here that we could potentially change to a TooManyRedirects test case. Although I'd probably be happy to accept the PR either way.

Anyone else got feedback on this?

@lovelydinosaur
lovelydinosaur merged commit efe9f61 into encode:master Feb 24, 2020
@lovelydinosaur

Copy link
Copy Markdown
Member

Fab, thanks!

@lovelydinosaur

Copy link
Copy Markdown
Member

Minor API change here, which is fine since we're already commited to the next release being 0.12.0

@florimondmanca florimondmanca mentioned this pull request Mar 4, 2020
3 tasks done
@lovelydinosaur lovelydinosaur mentioned this pull request Mar 5, 2020
Kludex referenced this pull request in pydantic/httpx2 May 11, 2026
Bumps [ruff](https://github.com/astral-sh/ruff) from 0.0.277 to 0.0.291.
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/BREAKING_CHANGES.md)
- [Commits](astral-sh/ruff@v0.0.277...v0.0.291)

---
updated-dependencies:
- dependency-name: ruff
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants