-
Notifications
You must be signed in to change notification settings - Fork 17
Channels compat #347
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
Channels compat #347
Conversation
@@ -3,7 +3,7 @@ repos: | |||
rev: 22.6.0 | |||
hooks: | |||
- id: black | |||
language_version: python3.9 | |||
language_version: python3.10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can revert this when merged, I don't have python 3.9. If you insist I can download.
@@ -110,8 +111,17 @@ def resolve(self, _next, root, info: GraphQLResolveInfo, *args, **kwargs): | |||
return _next(root, info, **kwargs) | |||
|
|||
|
|||
def channels_compat(context: StrawberryChannelsContext) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't put that in utils
since it is used only here. This is a function for readability.
class AsyncJSONWebTokenMiddleware(BaseJSONWebTokenMiddleware): | ||
async def resolve(self, _next, root, info: GraphQLResolveInfo, *args, **kwargs): | ||
if isinstance(info.context, StrawberryChannelsContext): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is the first entry point as far as our concerns am I right? Also this is not in BaseJSONWebTokenMiddleware
since channels won't use sync schema.
Things to discuss here:
|
…a extension's `resolve` not called while running `schema.subscribe`.
draft PR resolves #346
tests will be added when the channels pr will be merged.