-
Notifications
You must be signed in to change notification settings - Fork 78
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
DJANGO_DRAMATIQ_TASKS_NOT_WRITES, DJANGO_DRAMATIQ_TASKS_WRITES_ONLY #163
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,7 @@ | ||||||
from datetime import timedelta | ||||||
from typing import Optional | ||||||
|
||||||
from django.conf import settings | ||||||
from django.db import models | ||||||
from django.utils.functional import cached_property | ||||||
from django.utils.timezone import now | ||||||
|
@@ -10,9 +12,19 @@ | |||||
#: The database label to use when storing task metadata. | ||||||
DATABASE_LABEL = DjangoDramatiqConfig.tasks_database() | ||||||
|
||||||
DJANGO_DRAMATIQ_TASKS_NOT_WRITES = getattr(settings, "DJANGO_DRAMATIQ_TASKS_NOT_WRITES", []) | ||||||
DJANGO_DRAMATIQ_TASKS_WRITES_ONLY = getattr(settings, "DJANGO_DRAMATIQ_TASKS_WRITES_ONLY", []) | ||||||
|
||||||
|
||||||
class TaskManager(models.Manager): | ||||||
def create_or_update_from_message(self, message, **extra_fields): | ||||||
def create_or_update_from_message(self, message, **extra_fields) -> Optional['Task']: | ||||||
|
||||||
# black and write lists | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay |
||||||
if DJANGO_DRAMATIQ_TASKS_WRITES_ONLY and message.actor_name not in DJANGO_DRAMATIQ_TASKS_WRITES_ONLY: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Since you default to an empty list in the getattr method above, the first part of this conditional should not be necessary.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. False-like vals will not works this way. For example - None |
||||||
return None | ||||||
if DJANGO_DRAMATIQ_TASKS_NOT_WRITES and message.actor_name in DJANGO_DRAMATIQ_TASKS_NOT_WRITES: | ||||||
return None | ||||||
|
||||||
task, _ = self.using(DATABASE_LABEL).update_or_create( | ||||||
id=message.message_id, | ||||||
defaults={ | ||||||
|
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.
nitpick:
I dont believe we are using types in this project. Can you please remove this for consistency? 😄
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 suggest adding types everywhere, it always makes it easier to read the code. Compatibility will not be affected: in the library "Requirements Python 3.9+"