Skip to content

Fix detection of custom task set as class attribute with Django#9038

Merged
auvipy merged 4 commits intocelery:mainfrom
browniebroke:fix/custom-user-task-class-attr-django
May 27, 2024
Merged

Fix detection of custom task set as class attribute with Django#9038
auvipy merged 4 commits intocelery:mainfrom
browniebroke:fix/custom-user-task-class-attr-django

Conversation

@browniebroke
Copy link
Contributor

@browniebroke browniebroke commented May 22, 2024 •
edited
Loading

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

Description

Fix for feature added in

As reported in a comment on the pull request
, this feature breaks in case the user customised their task via a class attribute, and have Django installed:

class CeleryApp(Celery):
    registry_cls = 'tenant_schemas_celery.registry:TenantTaskRegistry'
    task_cls = 'tenant_schemas_celery.task:TenantTask'
  • Add a test to cover this use case
  • When such task is defined, don't use the built-in DjangoTask

@browniebroke browniebroke force-pushed the fix/custom-user-task-class-attr-django branch from c63fa78 to c6b243d Compare May 22, 2024 13:10
@browniebroke browniebroke changed the title Fix custom task set as class attribute with Django not being used Fix detection of custom task set as class attribute with Django May 22, 2024
@browniebroke browniebroke mentioned this pull request May 22, 2024
Merged
3 tasks
@browniebroke browniebroke force-pushed the fix/custom-user-task-class-attr-django branch 2 times, most recently from 33fbc16 to 96ca534 Compare May 22, 2024 13:37
@codecov
Copy link

codecov bot commented May 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.80%. Comparing base (448ada2) to head (7db5111).
Report is 63 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #9038   +/-   ##
=======================================
  Coverage   77.80%   77.80%           
=======================================
  Files         150      150           
  Lines       18682    18682           
  Branches     3194     3194           
=======================================
  Hits        14536    14536           
  Misses       3854     3854           
  Partials      292      292
Flag Coverage Δ
unittests 77.78% <100.00%> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

maciej-gol
maciej-gol reviewed May 22, 2024
View reviewed changes
@browniebroke browniebroke marked this pull request as ready for review May 22, 2024 14:04
@Nusnus Nusnus force-pushed the fix/custom-user-task-class-attr-django branch from 96ca534 to 0d8936d Compare May 23, 2024 00:18
@auvipy
Copy link
Member

auvipy commented May 26, 2024

Brian, do you intend to do more changes here?

@browniebroke
Copy link
Contributor Author

browniebroke commented May 26, 2024

Assuming you meant Bruno (me) 😁, no I don't intend to add any more changes. Should be good to go from my perspective.

@auvipy
Copy link
Member

auvipy commented May 27, 2024

Sorry sorry! less attention to detail problem! :D

@auvipy auvipy added this to the 5.4.x milestone May 27, 2024
@auvipy auvipy merged commit ae5eeb0 into celery:main May 27, 2024
@browniebroke browniebroke deleted the fix/custom-user-task-class-attr-django branch May 27, 2024 13:38
@glennmatthews glennmatthews mentioned this pull request Jun 11, 2024
Closed
@glennmatthews glennmatthews mentioned this pull request Jun 28, 2024
Merged
1 task
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

+1 more reviewer

@maciej-gol maciej-gol maciej-gol left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: Django PR Type: Bugfix PR Type: Enhancement

Projects

None yet

Milestone

5.4.x

Development

Successfully merging this pull request may close these issues.

3 participants

@browniebroke @auvipy @maciej-gol