Fix AttributeError when pool_cls is a string in Django fixups#10043
Fix AttributeError when pool_cls is a string in Django fixups#10043maycuatroi1 wants to merge 1 commit into
Conversation
When running Flower with Celery 5.6.1, an AttributeError is raised in celery/fixups/django.py because pool_cls can be a string (e.g., 'prefork') instead of a class object, and strings don't have a __module__ attribute. This fix checks if pool_cls is a string before accessing __module__, handling both cases correctly. Fixes celery#10042
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10043 +/- ##
=======================================
Coverage 88.41% 88.41%
=======================================
Files 153 153
Lines 19363 19366 +3
Branches 2226 2227 +1
=======================================
+ Hits 17120 17123 +3
Misses 1943 1943
Partials 300 300
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an AttributeError that occurs when Flower (or other tools) initialize Celery workers where pool_cls is still a string (e.g., 'prefork') instead of a resolved class object. The error occurred in the Django fixup when attempting to access the __module__ attribute on the string.
Key Changes
- Modified
_close_database()incelery/fixups/django.pyto check ifpool_clsis a string before accessing.__module__ - Added comprehensive test coverage for string and class types of
pool_cls
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
celery/fixups/django.py |
Added type checking for pool_cls to handle both string and class types when determining if the pool is prefork |
t/unit/fixups/test_django.py |
Added test case covering string pool_cls scenarios including 'prefork', 'threads', and module paths |
| pool_cls = self.worker.pool_cls | ||
| if isinstance(pool_cls, str): | ||
| is_prefork = "prefork" in pool_cls | ||
| else: | ||
| is_prefork = "prefork" in pool_cls.__module__ |
There was a problem hiding this comment.
The fix correctly handles the case where pool_cls is a string. However, similar code exists in other files that may have the same issue. Were those locations reviewed?
celery/contrib/testing/worker.pyline 45:if self.pool_cls.__module__.split('.')[-1] == 'prefork':celery/worker/components.pyline 197:if w.pool_cls.__module__.endswith(('gevent', 'eventlet')):
Should we apply the same fix to those locations in this PR or in a follow-up?
|
Closing in favor of #10045 which fixes the root cause (recursive WorkController instantiation). My PR only addressed the symptom. |
Summary
Fix
AttributeError: 'str' object has no attribute '__module__'when running Flower with Celery 5.6.1.Changes
celery/fixups/django.pyto check ifpool_clsis a string before accessing.__module__test_close_database_pool_cls_as_stringto cover this scenarioRoot Cause
In
_close_database()method at line 220, the code assumespool_clsis a class with a__module__attribute:However, when Flower initializes,
pool_clscan still be a string (e.g.,'prefork') instead of the resolved class object.Fix
Error Traceback (before fix)
Test Plan
test_close_database_pool_cls_as_stringFixes #10042