Skip to content

Improve robustness of live HTTP/2 test - #512

Merged
florimondmanca merged 1 commit into
masterfrom
fix/http2-live-test
Nov 7, 2019
Merged

Improve robustness of live HTTP/2 test#512
florimondmanca merged 1 commit into
masterfrom
fix/http2-live-test

Conversation

@florimondmanca

@florimondmancaflorimondmanca commented Nov 6, 2019

Copy link
Copy Markdown
Contributor

Right now the master build is failing because the live HTTP/2 request made to nghttp2.org is timing out, probably because the website is suffering some issues right now.

This PR makes the test xfail on some predictable failure cases.

Note that the socket.gaierror block is not ideal — when resolving #349 we should switch to whichever HTTPX-native exception is introduced.

@florimondmancaflorimondmanca added the tooling Changes to our CI/CD, tests setup, etc. label Nov 6, 2019
@florimondmanca

Copy link
Copy Markdown
ContributorAuthor

I triggered a new build on master, which is now fixed, and the build here passed without xfailing. However I was bitten by the issue in #510, so I'd say this change still has value.

@yeraydiazdiazyeraydiazdiaz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Though I have my reservations on the fact we need this external service for something perfectly achievable on local tests, #412 seemed like it would be a good fit for this case but it's gone a bit stale?

@florimondmanca

Copy link
Copy Markdown
ContributorAuthor

@yeraydiazdiaz Yes, though the main concern for #412 was possible overhead associated to using a non-Encode project for one single test. Uvicorn HTTP/2 development might be starting again soon ( Kludex/uvicorn#47), though there are no guarantees when that will land.

@yeraydiazdiaz

Copy link
Copy Markdown
Contributor

Right, I just feel I'd rather have a dependency on a non-Encode project for a while, potentially until HTTP/2 support lands in Uvicorn, than having a dependency on an external service. 🙂

@florimondmanca
florimondmanca merged commit 586acdd into masterNov 7, 2019
@florimondmanca
florimondmanca deleted the fix/http2-live-test branch November 7, 2019 23:35
Kludex referenced this pull request in pydantic/httpx2 May 11, 2026
Fixes#512
Co-authored-by: Tom Christie <tom@tomchristie.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

toolingChanges to our CI/CD, tests setup, etc.

Development

Successfully merging this pull request may close these issues.

2 participants

@florimondmanca@yeraydiazdiaz