-
-
Notifications
You must be signed in to change notification settings - Fork 842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stricter typing for request parameters. #3176
Comments
Thanks @karpetrosyan
So... I'm not necessarily(?) convinced that's the outcome I'd like to see here. For example, switching to the following* seems clearer to me... url : httpx.URL | str,
headers: httpx.Header | Mapping[str, str] | Sequence[Tuple[str, str]],
params: httpx.QueryParams | Mapping[str, str] | Sequence[Tuple[str, str]],
... * useful starting point for discussion |
I'm curios why not just expose from httpx.types import HeaderTypes, QueryParamsTypes, URLTypes
...
url : URLTypes | str,
headers: HeaderTypes | Mapping[str, str] | Sequence[Tuple[str, str]],
params: QueryParamsTypes | Mapping[str, str] | Sequence[Tuple[str, str]],
... |
Mostly I think these types end up obscuring the actual API and are code smells that should be squished. |
This is not necessarily a request for a change in public API, more of a note to add to the list. |
Okay. That looks like a bad idea, I wouldn't do that. |
@tomchristie, would you have an alternative proposal on how to do it efficiently? Other than patching each one of those methods, there didn't seem to be an obvious way to work around that. |
I suppose the point of clarity is to be aware that we might break that.
I suppose it depends on exactly what you're instrumenting. The |
How else can we resolve this problem? |
In my opinion types like UseClientDefault should be public as they're part of the public API and there's no way around it. I'd like my library (that wraps httpx) to be able to annotate functions properly. Convenience types like HeaderTypes should either be public or not used in the public API. Right now they are public API - if you change them, you're changing the public API`. I find the current setup confusing. |
Thoughts for slimming down the type interface a little, so that we no longer use type synonyms... HTTPX 1.0 becomes a little stricter on typing that previous versions, in these areas;
Here's an example of how the def post(
self,
url: URL | str,
*,
content: httpx.UploadContent | str | bytes | None = None,
data: httpx.FormData | Dict[str, str] | None = None,
files: httpx.FormFile | Dict[str, str] | None = None,
json: typing.Any | None = None,
params: httpx.QueryParams | Dict[str, str] | None = None,
headers: httpx.Headers | Dict[str, str] | None = None,
cookies: httpx.Cookies | Dict[str, str] | CookieJar | None = None,
auth: httpx.Auth | None = None,
follow_redirects: bool | None = None,
timeout: httpx.Timeout | None = None,
extensions: Dict[str, Any] | None = None
) |
I've retitled this to better reflect our design intent. |
This was original posted in issue 3170 #3170 (comment) for typing of data/params I'm ok if Data/params are encoded as And even user manually build url, it may still conflict with client arguments if httpx assume all params value is valid utf8 string |
Ref: encode/starlette#2534 (comment)
This issue was opened to resolve all kinds of problems where third-party packages may need to use private imports. We should definitely expose them to avoid problems like #3130 (comment).
I will include the package name along with its private imports (only starlette for now), so we can clearly identify what needs to be exposed.
Note that:
We can also add other packages as well to track all of them in this issue.
Starlette
The text was updated successfully, but these errors were encountered: