Skip to content

Fix httpx private api usage#2534

Closed
karpetrosyan wants to merge 6 commits intomasterfrom
fix-httpx-private-api-usage
Closed

Fix httpx private api usage#2534
karpetrosyan wants to merge 6 commits intomasterfrom
fix-httpx-private-api-usage

Conversation

@karpetrosyan
Copy link
Contributor

Refs: encode/httpx#3130

In some places, starlette uses imports from the private API, such as httpx._client, which will not work after mere of encode/httpx#3130.

@karpetrosyan karpetrosyan mentioned this pull request Mar 3, 2024
@Kludex
Copy link
Owner

Kludex commented Mar 3, 2024

Actually, we use future annotations here. Nothing will break at runtime. 🥹🙏

@Kludex
Copy link
Owner

Kludex commented Mar 3, 2024

Is there any discussion about making types available on public modules?

@karpetrosyan
Copy link
Contributor Author

Is there any discussion about making types available on public modules?

USE_CLIENT_DEFAULT was already public, we can use it directly from httpx.

@lovelydinosaur
Copy link
Contributor

I'd suggest including the type definitions explicitly inside Starlette's codebase.

@Kludex
Copy link
Owner

Kludex commented Mar 5, 2024

I'd suggest including the type definitions explicitly inside Starlette's codebase.

Ok. 👍

@Kludex
Copy link
Owner

Kludex commented Mar 5, 2024

I'd suggest including the type definitions explicitly inside Starlette's codebase.

Ok. 👍

Wanna do it here @karpetrosyan ?

@karpetrosyan
Copy link
Contributor Author

Wanna do it here @karpetrosyan ?

Yep

@karpetrosyan
Copy link
Contributor Author

Do we really can add them into the starlette?
There are types that should be passed through the httpx, how we should handle them?

@Kludex
Copy link
Owner

Kludex commented Mar 9, 2024

For UseClientDefault we can't. It's a class, and is private. cc @tomchristie

@lovelydinosaur
Copy link
Contributor

Noted. I guess there's type(httpx.USE_CLIENT_DEFAULT)

@Kludex
Copy link
Owner

Kludex commented Mar 9, 2024

Noted. I guess there's type(httpx.USE_CLIENT_DEFAULT)

We can't use that for type annotation. 👀

@Kludex
Copy link
Owner

Kludex commented Apr 20, 2024

What's the plan here? @karpetrosyan

@karpetrosyan
Copy link
Contributor Author

What's the plan here? @karpetrosyan

I believe the best we can do here is to limit the upper version of HTTPX to the current version. Alternatively, we can maintain this import in HTTPX only for Starlette, but I don't believe this is a good idea. Is it critical that older versions of Starlette do not work with the most recent HTTPX?

@lovelydinosaur
Copy link
Contributor

Shall we raise an issue for "3rd party packages using private imports of httpx"? That issue should describe what private imports starlette is currently using and why, so we can resolve whatever is causing that.

@lovelydinosaur
Copy link
Contributor

(I say "3rd party" here because we shouldn't treat other encode packages differently from the rest of the ecosystem. Starlette's use of some private importing will be a really helpful thing for us to work through what we're getting wrong. I expect we need to think a bit more carefully about our typing and API expectations, let's see)

@karpetrosyan
Copy link
Contributor Author

I have raised https://github.com/encode/httpx/issues/3176 to track all kinds of API needs in third-party packages.

@Kludex
Copy link
Owner

Kludex commented Jun 1, 2024

Since we are tracking the issue presented here on https://github.com/encode/httpx/issues/3176, I'll close this PR.

@Kludex Kludex closed this Jun 1, 2024
@Kludex Kludex deleted the fix-httpx-private-api-usage branch June 1, 2024 13:13
@lovelydinosaur lovelydinosaur mentioned this pull request Nov 24, 2024
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.

3 participants