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

feat: Support both sync and async in ComponentDependencyMiddleware #466

Merged
merged 6 commits into from
May 1, 2024

Conversation

TheSteveBurgess
Copy link
Contributor

Hi,

PR for converting the middleware to async as mentioned here (#464)

This is working fine for me both sync and async via Daphne.

@TheSteveBurgess
Copy link
Contributor Author

Well, that looks like I've broken something :)

Let me take a look why the tests are failing.

response = self.get_response(request)
response = self.process_response(request)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, maybe the issue is that we pass request here instead of response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That'd do it :) Hopefully it's working with that change

@JuroOravec
Copy link
Collaborator

@TheSteveBurgess thanks a lot for the PR! 🚀 It looks great! There's one typo, let's see if things work after that :)

TheSteveBurgess and others added 2 commits May 1, 2024 15:55
Co-authored-by: Juro Oravec <juraj.oravec.josefson@gmail.com>
Co-authored-by: Juro Oravec <juraj.oravec.josefson@gmail.com>
@JuroOravec
Copy link
Collaborator

I ran the tests locally, everything should be passing after fixing the function signatures

@JuroOravec JuroOravec changed the title Converted Middleware to Async feat: Support both sync and async in ComponentDependencyMiddleware May 1, 2024
@JuroOravec JuroOravec merged commit 0f34918 into EmilStenstrom:master May 1, 2024
6 checks passed
@TheSteveBurgess
Copy link
Contributor Author

Amazing! Thanks for that :)

@JuroOravec
Copy link
Collaborator

Merged! @TheSteveBurgess thanks a lot again :) I plan to release this as part of v0.70 together with #465 later today

@JuroOravec JuroOravec mentioned this pull request May 1, 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.

None yet

2 participants