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

Implemented request scoped translations #29

Conversation

AnderUstarroz
Copy link
Contributor

There is a serious issue on current implementation, the same Babel instance is shared globally, which means that the request of one user can change the language of another user request depending on race conditions. This PR takes care of scoping a single Babel instance per request.

@AnderUstarroz AnderUstarroz force-pushed the implement_request_scoped_babel_instances branch from 84b39b1 to 5552fe3 Compare February 9, 2024 13:56
Copy link

@caner-cetin caner-cetin left a comment

Choose a reason for hiding this comment

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

This PR would make fastapi-babel production ready by itself. thinking latest *stable release is more than 1 years ago, I think someone should fork with this PR and make a release in pip.

@Legopapurida
Copy link
Contributor

I have reviewed your changes. You have implemented good changes but there are a few considerations for request.state.
I will change the codes in the next version in another way.
we must implement a request local proxy to handle the gettext as a global variable throughout the files and packages (without passing the request object to do that.

@Legopapurida
Copy link
Contributor

I have tested the code in pull request and it does work fine.

Thanks for resolving this problem.

@Legopapurida Legopapurida merged commit 9a881e0 into Anbarryprojects:main Feb 25, 2024
@AnderUstarroz
Copy link
Contributor Author

I have reviewed your changes. You have implemented good changes but there are a few considerations for request.state. I will change the codes in the next version in another way. we must implement a request local proxy to handle the gettext as a global variable throughout the files and packages (without passing the request object to do that.

Sorry for not answering earlier... I saw that you remove the Singleton from core.py, then Babel.instance won't be needed anymore right? Thanks for taking the time to review the PR and making a new release 👍

@AnderUstarroz
Copy link
Contributor Author

Actually... are you sure we can just get rid of the Babel.instance? It seems to be used also in the make_gettext function.

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.

3 participants