Bug fix for config/i18n.py translator() using unsafe call get_current_request() #608

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

This fix assumes translationstring and chameleon will forward the context to translator() so that 'request' can be obtained from context instead of get_current_request() which is not thread safe when it is used with green threads like gevent. Pull request for translationstring and chameleon are being made so that the context with be passed in to translate().

unknown bug fix for concurrency issue with get_current_request() call. This f…
…ix requires translationstring to enable context for translator().
85506c0
Owner

mmerickel commented May 22, 2012

Why not pass the request into translator instead of context? What is context? I'm not familiar with a "context" in Pyramid that supports a dict-like interface containing the request. I guess there's the system object in the renderer?

On a side-note, using Pyramid with gevent requires monkey patching the threading module, at least currently. There is not really another way to guarantee that the public API get_current_request() will remain safe, and it's used in more places than just the translator.

cause in the flow translation flow, translationstring/init.py has a class ChameleonTranslate with translate(msgid, domain=None, mapping=None, context=None, target_language=None, default=None) which calls

result = translator(tstring) which could become result = translator(tstring, context)

Since context is already in translator() but not being use. This would be easiest to wire in and not changing the codebase too much. we submitted a pull request to translationstring project for this change as well.

Regarding get_current_request(), the function's doc itself had "it's almost always usually a mistake to use
get_current_request outside a testing context" and the only place that I can find real usage is in i18n.py the other places I found are debugtoolbar, and tests

Owner

bertjwregeer commented Sep 7, 2013

With the removal of the default template renderers from Pyramid, this problem is solved as well, since that function no longer exists (other than possibly in pyramid_chameleon but @mcdonc is working on that).

See #1114 for more information regarding those changes.

Owner

mcdonc commented Sep 7, 2013

This code has been moved to https://github.com/Pylons/pyramid_chameleon, although even there it will get_current_request for the foreseeable future. I'm going to close this issue, because I'm not sure messing around with this is worth having to change translationstring too. It's more or less ok how it is.

mcdonc closed this Sep 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment