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

Pass endpoint dependencies to Jinja context via request.state #6

Closed
wants to merge 1 commit into from
Closed

Pass endpoint dependencies to Jinja context via request.state #6

wants to merge 1 commit into from

Conversation

hasansezertasan
Copy link
Contributor

Related to #4

What is request.state and why am I using it?

https://github.com/encode/starlette/blob/433da65fc1431754dae0c3c87290b0421aa4a6f9/starlette/datastructures.py#L680-L685

Request State documentation says: An object that can be used to store arbitrary state.

I saw many implementations using the request.state to carry database sessions, current user, client sessions, etc. I think it's the best way to pass endpoint dependencies to the Jinja context without affecting anything.

@volfpeter
Copy link
Owner

What if someone already uses depends in there? It may be unlikely, but it's still not something a package should do. Also, its usage wouldn't be obvious to people, because its a bit of hidden magic. Explicit is better than implicit. I'll post my recommended solution in the issue.

@hasansezertasan
Copy link
Contributor Author

What if someone already uses depends in there? It may be unlikely, but it's still not something a package should do.

I disagree, there are quite a lot of (popular) packages that follow this approach but you are right, depends might be used by another package, and still, we are passing that data to the Jinja Template file and returning the HTML response.

Also, its usage wouldn't be obvious to people, because its a bit of hidden magic. Explicit is better than implicit. I'll post my recommended solution in the issue.

You are right about that but we can document it, the only advantage of this approach is to provide a way to access the dependencies without overriding the Jinja class.

@hasansezertasan
Copy link
Contributor Author

I'd like to mention this comment from the author of starlette: encode/starlette#659 (comment)

The recommended way is this: "For per-request information you can use request.state to store any additional information."

Other related issues:

So this is actually the recommended way of doing such a thing.

@volfpeter
Copy link
Owner

This may be the recommended way for certain use-cases in application development (always opinionated code and always in the hand of its developer), but I don't think it's good behavior for a Python package, because it could break applications.

@hasansezertasan
Copy link
Contributor Author

hasansezertasan commented Feb 7, 2024

I also want to highlight these resources:

I see that this is the "official" recommendation from the Starlette dev team. I also want to highlight some third-party-extensions that use request.state.something to store pre-request state/data.

Some other resources:

I believe there is no harm in using request.state._depends and update the README accordingly (in case someone uses that keyword for something else, it'll be a warning).

In the end, what do application and request context even exist for?

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.

2 participants