Skip to content

feat: add periodic_task_name#261

Merged
auvipy merged 5 commits intocelery:masterfrom
lvelvee:patch-periodic-task-name
Dec 15, 2021
Merged

feat: add periodic_task_name#261
auvipy merged 5 commits intocelery:masterfrom
lvelvee:patch-periodic-task-name

Conversation

@lvelvee
Copy link
Copy Markdown

@lvelvee lvelvee commented Nov 18, 2021

@lvelvee
Copy link
Copy Markdown
Author

lvelvee commented Nov 18, 2021

related: https://github.com/lvelvee/django-celery-beat/tree/feat-periodic-task-name

auvipy
auvipy requested changes Nov 18, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

please check the test failures

@lvelvee lvelvee changed the title feta: add periodic_task_name feat: add periodic_task_name Nov 18, 2021
@lvelvee lvelvee requested a review from auvipy November 18, 2021 14:45
@auvipy
Copy link
Copy Markdown
Member

auvipy commented Nov 18, 2021

some documentation change is require i guess

@auvipy
Copy link
Copy Markdown
Member

auvipy commented Nov 22, 2021

I am curious what extra benefits we get by adding this? what the existing 2 packages are missing?

@lvelvee
Copy link
Copy Markdown
Author

lvelvee commented Nov 23, 2021

I am curious what extra benefits we get by adding this? what the existing 2 packages are missing?

If a CeleryTask may correspond to more than one PeriodicTask, it will not be possible to distinguish the source of the PeriodicTask because django-celery-results lacks the name field of the PeriodicTask.

For example, if I have two PeriodicTask named periodic_a and periodic_b, both of which call the test task of celery at regular intervals, without this patch, the results will only be known as the results of test, and there will be no way to distinguish whether the task originated from periodic_a or periodic_b.

@lvelvee
Copy link
Copy Markdown
Author

lvelvee commented Dec 15, 2021

@auvipy Is it still possible to be merged?

auvipy
auvipy approved these changes Dec 15, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

i guess some change in celery beat is also required?

@auvipy auvipy merged commit 5e67a31 into celery:master Dec 15, 2021
@lvelvee
Copy link
Copy Markdown
Author

lvelvee commented Dec 15, 2021

I'll make the pr later

@lvelvee lvelvee mentioned this pull request Dec 15, 2021
Merged
auvipy pushed a commit to celery/django-celery-beat that referenced this pull request Dec 17, 2021
feat: add periodic_task_name in favor of celery/django-celery-results…
77a6ccf 
…#261 (#477)

* feat: add periodic_task_name

* docs and tests

* more detail docs

Co-authored-by: mba <mba@host.local>
@lvelvee lvelvee deleted the patch-periodic-task-name branch December 17, 2021 08:02
@auvipy auvipy linked an issue Dec 22, 2021 that may be closed by this pull request
Store task name ? #7
Closed
@KOliver94 KOliver94 mentioned this pull request Jan 9, 2022
Closed
@KOliver94 KOliver94 mentioned this pull request Jan 26, 2022
Merged
@awais786 awais786 mentioned this pull request May 13, 2022
Merged
@peterfarrell peterfarrell mentioned this pull request Sep 26, 2022
Closed
Oleh929216 added a commit to Oleh929216/Django-beat that referenced this pull request Jan 30, 2025
@Oleh929216
feat: add periodic_task_name in favor of celery/django-celery-results…
ea42f05 
…#261 (#477)

* feat: add periodic_task_name

* docs and tests

* more detail docs

Co-authored-by: mba <mba@host.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@auvipy auvipy auvipy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Store task name ?

2 participants

@lvelvee @auvipy