search

encode / starlette

The little ASGI framework that shines. 🌟
https://www.starlette.io/
BSD 3-Clause "New" or "Revised" License
10.21k stars 930 forks source link

Private imports from `httpx`. #2673

Open tomchristie opened 2 months ago

tomchristie commented 2 months ago

Prompted by https://github.com/encode/httpx/discussions/3287

starlette is currently using private imports from httpx...

https://github.com/encode/starlette/blob/c78c9aac17a4d68e0647252310044502f1b7da71/starlette/testclient.py#L500-L513

These are broken against the latest minor point release, which includes this (internal) refactoring... https://github.com/encode/httpx/pull/3245

It's possible that the path of least resistance for the short term is for httpx to re-include URLTypes, tho really we need to be making sure that downstream packages are not using private imports.

Some prior discussion of this also at #2534.

tomchristie commented 2 months ago

This comment makes a good case that this is an httpx issue... https://github.com/encode/httpx/issues/3176#issuecomment-2226886331

Kludex commented 1 month ago

@tomchristie , what do you suggest here?

Kludex commented 1 month ago

Maybe we can remove httpx as a dependency for the TestClient? It's always been confusing to install httpx for the testclient module for Starlette users. What do you think? Would that be bad for httpx?

tomchristie commented 1 month ago

Maybe we can remove httpx as a dependency for the TestClient?

Depends on exactly what you mean. It'd be an API breaking change, since it currently returns httpx types.

Something I do like about the existing test client is that it's mirroring using an actual Python client library to interact with the app.

One option could be a similar approach as eg. https://www.starlette.io/database/ - Don't have a dependency, but do document how to use httpx as a test client. (We'd need to work through any functional additions that Starlette's TestClient provides. WebSockets anyone?)

Would that be bad for httpx?

We should aim for streamlined. (Consistent project guidelines, docs, tooling approaches etc.) We shouldn't aim for preferential treatment.

Kludex commented 1 month ago

Let's go back to the original issue. What should Starlette do? Should httpx make types publich?

tomchristie commented 1 month ago

Really I'd like to tighten up the type definitions throughout, so that they're not needed...

Eg... aiming for this type of thing...

    def request(
        self,
        method: str,
        url: URL | str,
        *,
        content: typing.Any = None,
        data: typing.Any = None,
        files: typing.Any = None,
        json: typing.Any = None,
        params: QueryParams | Mapping[str, str] | None = None,
        headers: Headers | Mapping[str, str] | None = None,
        cookies: Cookies | CookieJar | Mapping[str, str] | None = None,
        auth: Auth | None = None,
        follow_redirects: bool | None = None,
        allow_redirects: bool | None = None,
        timeout: Timeout | float | None = None,
        extensions: dict[str, typing.Any] | None = None,
    )
Kludex commented 1 month ago

What about the USE_CLIENT_DEFAULT constant?

Also, I don't like that data, content, files and json have Any.