Skip to content
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

Use more permissible types in ASGIApp #3109

Merged
merged 3 commits into from
Feb 23, 2024
Merged

Conversation

jonfinerty
Copy link
Contributor

@jonfinerty jonfinerty commented Feb 22, 2024

Closes #3111

MutableMapping and Awaitable are slightly more permissible types (I think, as they allow the previous Dict and Coroutine types) but more importantly they match up to Starlettes tpyes

Summary

Hello! When upgrading to the latest version of Httpx we found that our test client was producing types errors (but fine at run time), we use FastAPI which is Starlette under the hood:

AsyncClient(transport=ASGITransport(app=app), base_url="http://test")

Argument of type "FastAPI" cannot be assigned to parameter "app" of type "_ASGIApp" in function "__init__"
  Type "FastAPI" cannot be assigned to type "_ASGIApp"
    Parameter 3: type "_Send" cannot be assigned to type "Send"
      Type "_Send" cannot be assigned to type "Send"
        Parameter 1: type "Message" cannot be assigned to type "Dict[str, Any]"
          "Message" is incompatible with "Dict[str, Any]"Pylance[reportArgumentType](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportArgumentType)

This might be a bug in pylance not understanding the nested mutable mappings and their compatibililty with dicts, but this change makes httpx more flexible with other ASGI apps, and matches for example Starlette's ASGI app types: https://github.com/encode/starlette/blob/7533b61a34f8b0dcb5fc35c717d458439f0baf18/starlette/types.py#L10

So the consistency might be worth it?

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change. (covered by existing tests)
  • I've updated the documentation accordingly.

@jonfinerty jonfinerty changed the title Use the type.MutableMapping instead of Dict in ASGIApp types Use more permissible types in ASGIApp Feb 22, 2024
@tomchristie
Copy link
Member

Yep. Seems reasonable enough... possibly worth a CHANGELOG.md item. (Tho I think it'd be okay either way)

MutableMapping is a slightly more permissible type (allowing the previous Dict type) but matches up to Starlettes tpyes
@jonfinerty
Copy link
Contributor Author

changelog added!

CHANGELOG.md Outdated Show resolved Hide resolved
@tomchristie
Copy link
Member

Thanks, let's get this in.

@tomchristie tomchristie merged commit 4de1370 into encode:master Feb 23, 2024
5 checks passed
@antonwnk
Copy link

Just curious when this gets shipped into a version. Couldn't find any info on that.

Meanwhile I'll be using the deprecated AsyncClient(app=app...

@dcermak dcermak mentioned this pull request Mar 26, 2024
3 tasks
shepilov-vladislav pushed a commit to shepilov-vladislav/httpx that referenced this pull request Mar 28, 2024
* Use the type.MutableMapping instead of Dict

MutableMapping is a slightly more permissible type (allowing the previous Dict type) but matches up to Starlettes tpyes

* Update CHANGELOG.md

---------

Co-authored-by: Tom Christie <tom@tomchristie.com>
@jamestrousdale
Copy link

Is there any plan to release this change soon? Still getting hit with deprecation warnings, but if we update then mypy and pyright complain.

@mbeijen mbeijen mentioned this pull request Aug 21, 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.

0.27.0: replacing app=app with transport=ASGITransport(app) doesn't type check
4 participants