-
-
Notifications
You must be signed in to change notification settings - Fork 290
Add localization activation ability #275
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #275 +/- ##
==========================================
+ Coverage 82.06% 82.08% +0.01%
==========================================
Files 24 24
Lines 1344 1351 +7
==========================================
+ Hits 1103 1109 +6
- Misses 241 242 +1
Continue to review full report at Codecov.
|
django_rq/workers.py
Outdated
@@ -38,6 +39,11 @@ def get_worker_class(worker_class=None): | |||
return worker_class | |||
|
|||
|
|||
def activate_localization(): | |||
if getattr(settings, 'RQ_USE_LOCALIZATION', False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move this variable to DjangoRQ's settings.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
django_rq/workers.py
Outdated
@@ -38,6 +39,11 @@ def get_worker_class(worker_class=None): | |||
return worker_class | |||
|
|||
|
|||
def activate_localization(): | |||
if getattr(settings, 'RQ_USE_LOCALIZATION', False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can simplify this to if settings.RQ_USE_LOCALIZATION:
django_rq/settings.py
Outdated
@@ -25,3 +25,8 @@ | |||
|
|||
# Token for querying statistics | |||
API_TOKEN = getattr(settings, 'RQ_API_TOKEN', '') | |||
|
|||
# Activate locale for each task | |||
RQ_USE_LOCALIZATION = getattr(settings, 'RQ_USE_LOCALIZATION', False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this to RQ_USE_L10N
to keep it consistent with Django's naming scheme? RQ_LANGUAGE_CODE
defaults to Django's LANGUAGE_CODE
setting, should we also change this to default to Django's USE_L10N
setting?
django_rq/workers.py
Outdated
@@ -49,6 +55,7 @@ def get_worker(*queue_names, **kwargs): | |||
# normalize queue_class to what get_queues returns | |||
queue_class = queues[0].__class__ | |||
worker_class = get_worker_class(kwargs.pop('worker_class', None)) | |||
activate_localization() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simply do this (and delete activate_localization()
):
if settings.RQ_USE_LOCALIZATION:
activate(settings. RQ_LANGUAGE_CODE)
@derfenix reminder to update this PR |
If you're still interested in getting this in, please fix the failing tests and document this feature in |
I hope I will find time in the closest week. Thanks for patience
сб, 30 мар. 2019 г. в 05:42, Selwin Ong <notifications@github.com>:
… If you're still interested in getting this in, please fix the failing
tests and document this feature in README
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#275 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAgj2PfCOoG1AnpNiMaliDBHVq7AuYeyks5vbs8tgaJpZM4SMQJt>
.
|
Add ability to use default localization in tasks without manual activation for each task.