Skip to content

Fix eager tasks does not populate name field#8383

Merged
auvipy merged 3 commits intocelery:mainfrom
KOliver94:eager_task_no_name_bug
Jul 25, 2023
Merged

Fix eager tasks does not populate name field#8383
auvipy merged 3 commits intocelery:mainfrom
KOliver94:eager_task_no_name_bug

Conversation

@KOliver94
Copy link
Contributor

@KOliver94 KOliver94 commented Jul 20, 2023

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Multiple issues were reported both here (#7715) and in django-celery-results (celery/django-celery-results#289 & celery/django-celery-results#362) that when running in eager mode name of the tasks are not populated. After my investigation it seems to me that this field was just missed as it's not critical in terms of functionality. I added it and it seems to work.

Let me know what do you think.

Fixes #7715

@KOliver94 KOliver94 changed the title Eager task no name bug Eager tasks leave name field empty Jul 21, 2023
@KOliver94 KOliver94 changed the title Eager tasks leave name field empty Fix eager tasks does not populate name field Jul 21, 2023
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (78ab64e) 87.07% compared to head (8cbf382) 87.07%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8383   +/-   ##
=======================================
  Coverage   87.07%   87.07%           
=======================================
  Files         148      148           
  Lines       18491    18492    +1     
  Branches     3152     3152           
=======================================
+ Hits        16101    16102    +1     
  Misses       2110     2110           
  Partials      280      280           
Flag Coverage Δ
unittests 87.04% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/app/task.py 94.80% <100.00%> (ø)
celery/result.py 96.70% <100.00%> (+<0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@auvipy auvipy added this to the 5.3.x milestone Jul 24, 2023
Copy link
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.

that looks good to me! can we add some sort of integration test for this as well? some manual acknowledgement would be also great

@KOliver94
Copy link
Contributor Author

Thank you @auvipy! I took a look at the existing tests but I didn't find any for similar case so I'm not sure what type of test can we provide for this. If you can show me some which you think can be used as starter and specify what you think should be tested, I'm happy to write some.

@auvipy
Copy link
Member

auvipy commented Jul 25, 2023

I think we are safe to go as is for now

@auvipy auvipy merged commit 1c36387 into celery:main Jul 25, 2023
@KOliver94 KOliver94 deleted the eager_task_no_name_bug branch July 25, 2023 15:05
@shifenhutu
Copy link
Contributor

can you release a version ?

@auvipy
Copy link
Member

auvipy commented Aug 13, 2023

it will be included in next point release very soon

@auvipy
Copy link
Member

auvipy commented Sep 2, 2023

we got a bug / regression report here #8472 can you please verify?

auvipy added a commit that referenced this pull request Sep 2, 2023
auvipy added a commit that referenced this pull request Sep 2, 2023
@KOliver94 KOliver94 restored the eager_task_no_name_bug branch September 2, 2023 12:06
@KOliver94 KOliver94 deleted the eager_task_no_name_bug branch September 4, 2023 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EagerResult doesn't seem to poplate name

3 participants