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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

DJANGO_DRAMATIQ_TASKS_NOT_WRITES, DJANGO_DRAMATIQ_TASKS_WRITES_ONLY #163

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d81c11b
DJANGO_DRAMATIQ_TASKS_NOT_WRITES, DJANGO_DRAMATIQ_TASKS_WRITES_ONLY
ikvk Apr 27, 2024
69e7766
resolve
ikvk Jan 30, 2025
f50de6c
resolve
ikvk Jan 30, 2025
2e6a63e
resolve
ikvk Jan 30, 2025
826b14f
rename, resolve, doc
ikvk Jan 30, 2025
1b84b15
resolve...
ikvk Jan 30, 2025
320dfc3
resolve...
ikvk Jan 30, 2025
f7326ad
resolve...
ikvk Jan 30, 2025
649722e
del res type for create_or_update_from_message
ikvk Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ DRAMATIQ_RESULT_BACKEND = {
}
```

You can specify which actors to write to the database with "black" and "white" lists:

``` python
DJANGO_DRAMATIQ_TASKS_NOT_WRITES = ['actor_name_that_excluded']
DJANGO_DRAMATIQ_TASKS_WRITES_ONLY = ['actor_name_that_writes_only1', 'actor_name_that_writes_only2']
```


## Usage

Expand Down
14 changes: 13 additions & 1 deletion django_dramatiq/models.py
View file
Open in desktop
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
Expand All @@ -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']:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick:

Suggested change
def create_or_update_from_message(self, message, **extra_fields) -> Optional['Task']:
def create_or_update_from_message(self, message, **extra_fields):

I dont believe we are using types in this project. Can you please remove this for consistency? 😄

Copy link
Author

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+"

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a valid suggestion! However, adding types to just one function would be inconsistent. If we decide to introduce type annotations, it would require static type checking and should be applied across the major APIs of the library. For now, we'll stick to the existing convention in this PR and keep it without types.

Copy link
Author

Choose a reason for hiding this comment

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

Done


# black and write lists
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick:

Suggested change
# black and write lists

Copy link
Author

Choose a reason for hiding this comment

The 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:
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
if DJANGO_DRAMATIQ_TASKS_WRITES_ONLY and message.actor_name not in DJANGO_DRAMATIQ_TASKS_WRITES_ONLY:
if message.actor_name not in DJANGO_DRAMATIQ_TASKS_WRITES_ONLY:

Copy link
Author

@ikvk ikvk Dec 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The 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={
Expand Down