NotificationsYou must be signed in to change notification settings
Fork1.9k
Star4.6k

Django channels stubs#13939

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

huynguyengl99 wants to merge10 commits intopython:main

base:main

Choose a base branch

fromhuynguyengl99:feature/channels-stubs

Open

Django channels stubs#13939

huynguyengl99 wants to merge10 commits intopython:mainfromhuynguyengl99:feature/channels-stubs

Conversation

Copy link

Contributor

huynguyengl99 commentedMay 4, 2025•
edited
Loading

Stubs package forDjango channels

Ref:

Stubs package for Django Channels #14020

This comment has been minimized.

1 similar comment

This comment has been minimized.

huynguyengl99 force-pushed thefeature/channels-stubs branch from040b12e to2edbaf3Compare

May 4, 2025 17:16

This comment has been minimized.

1 similar comment

This comment has been minimized.

huynguyengl99 mentioned this pull request

May 4, 2025

Add asgiref to allowlisttypeshed-internal/stub_uploader#173

Merged

Copy link

ContributorAuthor

huynguyengl99 commentedMay 4, 2025

Hi@sobolevn,
I’ve created stubs to add type hint support for django-channels. Sorry for the tag, but if you have some time, could you please take a look and share any feedback or suggestions? I’d really appreciate your input!

This comment has been minimized.

sobolevn reviewed

May 4, 2025

View reviewed changes

Copy link

Member

sobolevn left a comment

Choose a reason for hiding this comment

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

I havesome concerns about addingdjango-channels totypeshed. I would like to discuss them in detail here:

Sincedjango-stubs does not supportpyright and other type checkers - what will be the result of adding these types here, wherepyright andpytype are also included in the CI?
What about different django versions? Doesdjango-channels have any difference between them? We (django-stubs) only support the latest django version
Doesdjango-channels need any extra features? Would it benefit from a custom mypy plugin?

Copy link

Contributor

Tatsh commentedMay 4, 2025•
edited
Loading

There is a very good example of usage of Channelshere.

I havesome concerns about addingdjango-channels totypeshed. I would like to discuss them in detail here:
Sincedjango-stubs does not supportpyright and other type checkers - what will be the result of adding these types here, wherepyright andpytype are also included in the CI?

From the way I've been using Channels, it will not pull in anything that will not work properly with Pyright such as an app's models.

What about different django versions? Doesdjango-channels have any difference between them? We (django-stubs) only support the latest django version

They only check forDjango>=4.2.

Doesdjango-channels need any extra features? Would it benefit from a custom mypy plugin?

I do not believe so.

It is a somewhat active project so I think the typing should go in the project itself rather than here.

sobolevn reviewed

May 4, 2025

View reviewed changes

Copy link

Member

sobolevn left a comment

Choose a reason for hiding this comment

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

(not a full review)

stubs/channels/METADATA.toml OutdatedShow resolvedHide resolved

stubs/channels/channels/testing/http.pyiShow resolvedHide resolved

stubs/channels/channels/testing/live.pyi OutdatedShow resolvedHide resolved

Copy link

ContributorAuthor

huynguyengl99 commentedMay 5, 2025

@sobolevn

Sincedjango-stubs does not supportpyright and other type checkers — what will be the result of adding these types here, wherepyright andpytype are also included in the CI?

I agree with@Tatsh, and I am creating an extension package for Channels. It looks likedjango-stubs partially supports Pyright, rather than having no support at all. So for the Channels stubs themselves, it should work fine because they don’t currently require the mypy plugin — meaning the other type checkers should handle it well.

What about different Django versions? Doesdjango-channels have any differences between them? We (django-stubs) only support the latest Django version.

As I mentioned in your code feedback, I think there’s no significant difference between versions because Channels only relies on some stable Django imports. I will check and aim to support at least Django 4.2 if possible. For that, I think requiringdjango-stubs >=4.2 would be the best for compatibility. Is it right or should I just use the latest 5.2 for django stubs?

Doesdjango-channels need any extra features? Would it benefit from a custom mypy plugin?

I think not. However, one useful addition to the maindjango-channels package could be something likeChannelScope, which defines additional keys that can be accessed. This way, if users want to extend the scope, they can extend that type and annotate it in their code.

@Tatsh, I used to think about adding type annotations directly to Channels instead of creating a stubs package, but there’sa comment from carltongibson mentioning that type hints have been rejected in Django. So, Channels will likely follow that approach — that’s the main reason for creating the stubs.

This comment has been minimized.

huynguyengl99 force-pushed thefeature/channels-stubs branch from24e066b toe80bd03Compare

May 5, 2025 05:44

This comment has been minimized.

huynguyengl99 force-pushed thefeature/channels-stubs branch frome80bd03 to0f45154Compare

May 5, 2025 10:31

This comment has been minimized.

huynguyengl99 added6 commits

May 12, 2025 11:00

Django channels stubs

f6f330b

Fix alias type for python 3.9

41cf2ba

Correct some Any type channel layer

7c38b21

Add type_check_only, TODO comment, and constrain django-stubs version

b38d9b4

Add type_check_only for channels consumer and testing http

76842c8

Add stubtest

4690c72

huynguyengl99 force-pushed thefeature/channels-stubs branch from0f45154 to4690c72Compare

May 12, 2025 04:00

This comment has been minimized.

huynguyengl99 mentioned this pull request

May 12, 2025

Stubs package for Django Channels#14020

Open

sobolevn reviewed

May 12, 2025

View reviewed changes

Copy link

Member

sobolevn left a comment

Choose a reason for hiding this comment

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

(not a full review)

stubs/channels/channels/consumer.pyi

Comment on lines +28 to +29

		def get_handler_name(message: dict[str, Any]) -> str: ...
		@type_check_only

Copy link

Member

sobolevnMay 5, 2025

Choose a reason for hiding this comment

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

Suggested change

	defget_handler_name(message:dict[str,Any])->str: ...
	@type_check_only
	defget_handler_name(message:dict[str,Any])->str: ...

	@type_check_only

strange that ruff does not catch this.

Copy link

ContributorAuthor

huynguyengl99May 12, 2025

Choose a reason for hiding this comment

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

If you're referring to the spacing between get_handler_name and @type_check_only, that has been formatted byblack. Black removed the extra space between them (even though I agree with you that an extra space would be appropriate)

stubs/channels/channels/auth.pyi Outdated

		from .utils import _ChannelApplication

		@database_sync_to_async
		def get_user(scope: _ChannelScope) -> AbstractBaseUser \| AnonymousUser: ...

Copy link

Member

sobolevnMay 12, 2025

Choose a reason for hiding this comment

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

Can we just turn this intoasync def? Or will it trigger an error?

Copy link

ContributorAuthor

huynguyengl99May 12, 2025

Choose a reason for hiding this comment

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

It will work for theauth.pyi. However, there is one thing I have encountered in another file:stubs/channels/channels/consumer.pyi. If I migrate thedispatch function from being annotated bydatabase_sync_to_async toasync def, I face this error in the stub test:

error: channels.consumer.SyncConsumer.dispatch is not a functionStub: in file typeshed/stubs/channels/channels/consumer.pyi:55def (self: channels.consumer.SyncConsumer, message: builtins.dict[builtins.str, Any]) -> typing.Coroutine[Any, Any, None]Runtime: at line 118async def (self, message)

If I use database_sync_to_async here, it's ok. I don't know why there is this difference, so I chose to use database_sync_to_async. But I'm willing to migrate toasync def and putdispatch inallow_list.txt if you thinkasync def is better.

Copy link

ContributorAuthor

huynguyengl99May 12, 2025

Choose a reason for hiding this comment

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

I have migrated all functions inauth.pyi to useasync def but keptdispatch inconsumer.pyi usingdatabase_sync_to_async. Please let me know if this approach is correct, thank you.

stubs/channels/channels/auth.pyi Outdated

		def login(scope: _ChannelScope, user: AbstractBaseUser, backend: BaseBackend \| None = ...) -> None: ...
		@database_sync_to_async
		def logout(scope: _ChannelScope) -> None: ...
		def _get_user_session_key(session: _LazySession) -> Any: ...

Copy link

Member

sobolevnMay 12, 2025

Choose a reason for hiding this comment

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

Why do we need this protected member? please, add a comment. We generally don't included protected names, until they are special / required.

Copy link

ContributorAuthor

huynguyengl99May 12, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Got it, wasn't aware of that convention. I'll remove the protected functions that are purely internal. Thanks for pointing this out!

I also remove these protected methods fromChannelLayerManager that appear to be internal-only:

classChannelLayerManager:# ... other methods ...def_reset_backends(self,setting:str,**kwargs:Any)->None: ...def_make_backend(self,name:str,config:dict[str,Any])->BaseChannelLayer: ...```"

stubs/channels/channels/auth.pyi OutdatedShow resolvedHide resolved

srittau linked an issue

May 12, 2025

that may beclosed by this pull request

Stubs package for Django Channels#14020

Open

huynguyengl99 added2 commits

May 12, 2025 22:17

Migrate database_sync_to_async to async def for auth.pyi functions

712f388

Remove internal function type definitions

9ebfab5

This comment has been minimized.

collinanderson mentioned this pull request

May 12, 2025

Add type hintsdjango/channels#893

Closed

Copy link

ContributorAuthor

huynguyengl99 commentedMay 13, 2025

Hi@sobolevn, I have updated the code following your suggestions and left a small question related to the dispatch function in my above comment. I hope you can have a quick look at that to see if it's okay or not.

Remove internal function type definitions (more)

4c430ce

This comment has been minimized.

carltongibson reviewed

May 13, 2025

View reviewed changes

stubs/channels/channels/utils.pyi

Comment on lines +12 to +15

		class _MiddlewareProtocol(Protocol):
		def __init__(self, args: Any, *kwargs: Any) -> None: ...
		async def __call__(self, scope: Any, receive: Any, send: Any) -> Any: ...

Copy link

carltongibsonMay 13, 2025

Choose a reason for hiding this comment

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

This isjust theASGI2Protocol fromasgiref.typing, which is already part ofASGIApplication no?

Copy link

ContributorAuthor

huynguyengl99May 13, 2025

Choose a reason for hiding this comment

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

No, the ASGI2Protocol class is missing the scope param incall:

classASGI2Protocol(Protocol):def__init__(self,scope:Scope)->None:        ...asyncdef__call__(self,receive:ASGIReceiveCallable,send:ASGISendCallable    )->None:        ...

Also the init of OriginValidator is a little bit different to the other middlewares (1 extra param), that's why I need to use *args and **kwargs instead:

classOriginValidator:application:_ChannelApplicationallowed_origins:Iterable[str|Pattern[str]]def__init__(self,application:_ChannelApplication,allowed_origins:Iterable[str|Pattern[str]])->None: ...asyncdef__call__(self,scope:_ChannelScope,receive:ASGIReceiveCallable,send:ASGISendCallable)->Any: ...

It's quite like the ASGI3Application, but we also have theinit function. I tried to match it with the middleware in sessions.pyi, security/websocket.pyi and middleware.pyi files, so I think I need to create a new protocol for that.

stubs/channels/channels/consumer.pyi OutdatedShow resolvedHide resolved

Correct Channels typo (plural)

f1333f8

Copy link

Contributor

github-actionsbot commentedMay 13, 2025

Diff frommypy_primer, showing the effect of this PR on open source code:

strawberry (https://github.com/strawberry-graphql/strawberry)- strawberry/channels/handlers/base.py:14: error: Cannot find implementation or library stub for module named "channels.consumer"  [import-not-found]- strawberry/channels/handlers/base.py:15: error: Cannot find implementation or library stub for module named "channels.generic.websocket"  [import-not-found]+ note: "__init_subclass__" of "object" defined here+ strawberry/channels/handlers/base.py:57: error: Incompatible types in assignment (expression has type "ChannelsLayer | None", base class "AsyncConsumer" defined the type as "BaseChannelLayer")  [assignment]+ strawberry/channels/handlers/base.py:66: error: Argument 1 of "dispatch" is incompatible with supertype "AsyncConsumer"; supertype defines the argument type as "dict[str, Any]"  [override]+ strawberry/channels/handlers/base.py:66: note: This violates the Liskov substitution principle+ strawberry/channels/handlers/base.py:66: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides+ strawberry/channels/handlers/base.py:77: error: Argument 1 to "dispatch" of "AsyncConsumer" has incompatible type "ChannelsMessage"; expected "dict[str, Any]"  [arg-type]+ strawberry/channels/handlers/base.py:209: error: Definition of "channel_layer" in base class "ChannelsConsumer" is incompatible with definition in base class "AsyncWebsocketConsumer"  [misc]- strawberry/channels/testing.py:13: error: Cannot find implementation or library stub for module named "channels.testing.websocket"  [import-not-found]+ strawberry/channels/testing.py:101: error: Incompatible types in assignment (expression has type "dict[str, Any]", variable has type "ConnectionAckMessage")  [assignment]+ strawberry/channels/testing.py:109: error: Incompatible types in assignment (expression has type "dict[str, Any]", variable has type "ConnectionAckMessage")  [assignment]+ strawberry/channels/testing.py:146: error: Incompatible types in assignment (expression has type "dict[str, Any]", variable has type "ConnectionInitMessage | ConnectionAckMessage | PingMessage | PongMessage | SubscribeMessage | NextMessage | ErrorMessage | CompleteMessage")  [assignment]+ strawberry/channels/handlers/ws_handler.py:69: error: Definition of "channel_layer" in base class "ChannelsConsumer" is incompatible with definition in base class "AsyncWebsocketConsumer"  [misc]+ strawberry/channels/handlers/ws_handler.py:133: error: Argument 1 to "run" of "AsyncBaseHTTPView" has incompatible type "GraphQLWSConsumer[Context, RootValue]"; expected "GraphQLWSConsumer[None, None]"  [arg-type]- strawberry/channels/handlers/http_handler.py:22: error: Cannot find implementation or library stub for module named "channels.db"  [import-not-found]- strawberry/channels/handlers/http_handler.py:23: error: Cannot find implementation or library stub for module named "channels.generic.http"  [import-not-found]+ strawberry/channels/handlers/http_handler.py:200: error: "BaseGraphQLHTTPConsumer" has no attribute "run"  [attr-defined]+ strawberry/channels/handlers/http_handler.py:207: error: Argument "headers" to "send_headers" of "AsyncHttpConsumer" has incompatible type "dict[bytes, bytes]"; expected "Iterable[tuple[bytes, bytes]] | None"  [arg-type]+ strawberry/channels/handlers/http_handler.py:360: error: Return type "Coroutine[Any, Any, ChannelsResponse | MultipartChannelsResponse]" of "run" incompatible with return type "ChannelsResponse" in supertype "SyncBaseHTTPView"  [override]- strawberry/channels/router.py:14: error: Cannot find implementation or library stub for module named "channels.routing"  [import-not-found]

huynguyengl99 mentioned this pull request

May 15, 2025

Argument 2 to "path" has incompatible type "_ASGIApplicationProtocol", with django url patternshuynguyengl99/channels-stubs#1

Open

Copy link

ContributorAuthor

huynguyengl99 commentedMay 18, 2025

Hi@sobolevn and@carltongibson, after implementing the fixes based on your feedback, do you think the code is ready to merge now, or should it be reviewed more carefully?

Labels

None yet

4 participants

Movatterモバイル変換

Uh oh!

Django channels stubs#13939

Are you sure you want to change the base?

Django channels stubs#13939

Uh oh!

Conversation

huynguyengl99 commentedMay 4, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

huynguyengl99 commentedMay 4, 2025

Uh oh!

This comment has been minimized.

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Tatsh commentedMay 4, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

huynguyengl99 commentedMay 5, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huynguyengl99May 12, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

huynguyengl99 commentedMay 13, 2025

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actionsbot commentedMay 13, 2025

Uh oh!

huynguyengl99 commentedMay 18, 2025

Uh oh!

Uh oh!

huynguyengl99 commentedMay 4, 2025•
edited
Loading

Tatsh commentedMay 4, 2025•
edited
Loading

huynguyengl99May 12, 2025•
edited
Loading