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

🐛 Allow async class methods as dependencies not decorated #10250

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danielfcollier
Copy link

@danielfcollier danielfcollier commented Sep 15, 2023

Found this problem: #10238

Which happens to be related with this issue: #10236

I could find the solution for async class methods in not decorated API routes.

Added tests to coverage the cases.

p.s.: still, missing to work with generators.

@danielfcollier danielfcollier force-pushed the allow-api-routes-async-class-mehtods-not-decorated branch from 7353741 to 5812a5c Compare September 15, 2023 04:24
@danielfcollier danielfcollier force-pushed the allow-api-routes-async-class-mehtods-not-decorated branch from 13a22eb to cc928d5 Compare September 15, 2023 05:03
@danielfcollier danielfcollier force-pushed the allow-api-routes-async-class-mehtods-not-decorated branch from a0c2f0f to d074265 Compare September 15, 2023 05:05
@Kludex
Copy link
Sponsor Collaborator

Kludex commented Sep 15, 2023

I think there's an utility function in starlette.utils for this.

@danielfcollier
Copy link
Author

danielfcollier commented Sep 15, 2023

I've checked Starlette, but the test cases for the add_route method are much simpler in scope and cases.

Theses changes I've checked going down with the debugger, I've seen only issues in the FastAPI layer. But, later I could check more to see if there is any relation between Starlette and the Generators part.

@alejsdev alejsdev added the p4 label Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants