Breakage when upgrading from Celery 2.5.3 -> 3.0.4

Commenting on my update to my Celery rant, @asksol asked me to post the Pylint results that made me question the claim of backwards compatibility.

(“@Asksol asked” — See what I did there? That’s alliteration. It’s a sign of a quality blog post. Ask for it by name.)

Again for the record, @asksol is a smart and friendly person. I know I wouldn’t last a day supporting a project the way he has supported Celery over multiple years. I’ve calmed down since yesterday, and I hope that something good results from my rant — if not for me, then for a future Celery user needing upgrade help. In his reply to my rant, @asksol describes some history and rationale for how he manages code change, and I encourage you to read it.

Here we go:

With django-celery 2.5.5, celery 2.5.3, Pylint 0.25.1, and a pylint.cfg that disables “W0511,R0904,R0801,R0903,C0302,R0922,R0914,R0902,C0111,R0912,R0915,I0011,W0232”, Pylint reports nothing.

When I upgrade to django-celery 3.0.4 and celery 3.0.4, Pylint reports:

$ fab pylint
[localhost] local: pylint --rcfile="/Users/johnd/Documents/dev-quisitor/dev/configs/pylint.cfg" ips_quisitor 
************* Module ips_quisitor.accounts.tasks
E0611: 2,0: No name 'PeriodicTask' in module 'celery.task'
************* Module ips_quisitor.admin_dash.admin
E1120: 49,8:CCAdmin.save_model: No value passed for parameter 'self' in function call
************* Module ips_quisitor.admin_dash.tasks
E0611: 3,0: No name 'Task' in module 'celery.task'
E0611: 3,0: No name 'task' in module 'celery.task'
E0611: 3,0: No name 'PeriodicTask' in module 'celery.task'
E1120:642,16:PatentBulkImportGroup._run: No value passed for parameter 'self' in function call
E1120:685,20:PatentBulkImportGroup._run: No value passed for parameter 'self' in function call
************* Module ips_quisitor.admin_dash.views
F0401: 2,0: Unable to import 'celery.task.control'
E0611: 2,0: No name 'control' in module 'celery.task'
E1120:180,20:IpsAdmin.ipsindex: No value passed for parameter 'self' in function call
E1120:191,20:IpsAdmin.ipsindex: No value passed for parameter 'self' in function call
E1120:201,20:IpsAdmin.ipsindex: No value passed for parameter 'self' in function call
E0611:666,8:IpsAdmin.task_control: No name 'conf' in module 'celery'
F0401:668,8:IpsAdmin.task_control: Unable to import 'celery.task.control'
E0611:668,8:IpsAdmin.task_control: No name 'control' in module 'celery.task'
E1120:683,25:IpsAdmin.task_control: No value passed for parameter 'self' in function call
E1120:687,25:IpsAdmin.task_control: No value passed for parameter 'self' in function call
E1120:691,25:IpsAdmin.task_control: No value passed for parameter 'self' in function call
E1120:793,8:IpsAdmin.assignmentimport: No value passed for parameter 'self' in function call
E1120:800,8:IpsAdmin.classbulkimport: No value passed for parameter 'self' in function call
E1120:801,8:IpsAdmin.classbulkimport: No value passed for parameter 'self' in function call
E1120:813,8:IpsAdmin.patenttextbulkimport: No value passed for parameter 'self' in function call
E1120:820,8:IpsAdmin.allbulkimport: No value passed for parameter 'self' in function call
E1120:821,8:IpsAdmin.allbulkimport: No value passed for parameter 'self' in function call
E1120:822,8:IpsAdmin.allbulkimport: No value passed for parameter 'self' in function call
E1120:823,8:IpsAdmin.allbulkimport: No value passed for parameter 'self' in function call
E1120:943,8:IpsAdmin.quartilecalculate: No value passed for parameter 'self' in function call
************* Module ips_quisitor.alerts.tasks
E0611: 4,0: No name 'PeriodicTask' in module 'celery.task'
************* Module ips_quisitor.common.tasks
F0401: 2,0: Unable to import 'celery.task.control'
E0611: 2,0: No name 'control' in module 'celery.task'
E0611: 3,0: No name 'TaskSet' in module 'celery.task'
************* Module ips_quisitor.corp_data.tasks
E0611: 2,0: No name 'task' in module 'celery.task'
************* Module ips_quisitor.export.tasks
E0611: 3,0: No name 'PeriodicTask' in module 'celery.task'
E0611: 3,0: No name 'Task' in module 'celery.task'
************* Module ips_quisitor.inference.tasks
E0611: 4,0: No name 'PeriodicTask' in module 'celery.task'
E0611: 4,0: No name 'Task' in module 'celery.task'
E1120:560,20:WordAnalysisUpdate.run: No value passed for parameter 'self' in function call
************* Module ips_quisitor.ipdc_updates.tasks
E0611: 3,0: No name 'PeriodicTask' in module 'celery.task'
E1120:807,20:IpdcUpdateTextImport.run: No value passed for parameter 'self' in function call
************* Module ips_quisitor.misc.tasks
E0611: 2,0: No name 'PeriodicTask' in module 'celery.task'

I’d like to make three comments about this.

The first is, I said Pylint threw “about 60” errors, when in fact it threw only 40. I apologize for this exaggeration. Although these are multiples of about 10 different error types, they occur in very different code paths. So in some cases the fix would not have simply been cutting & pasting.

The second is, @asksol’s assertion that sometimes our code didn’t import from the “right” location:

BTW: You mentioned moving celery.task.base to celery.app.task. Both of those are internal modules, the right location is `from celery import Task’.

If our code is doing that, it’s wrong, and I would want to fix it the next time I have to edit the code in question.

However, it’s worthwhile to consider why our code might import from the “wrong” location. We didn’t deliberately choose to do so! We’re smart developers. If we imported from the “wrong” location, it might be because we were confused about where the right location was. We thought it was the right location.

How did we make such mistakes? From a combination of confusing documentation, having to search through the source code to find what we thought was the right import target, and (if you wish) our own fallibility. My point here is, users importing from the wrong location is a problem at one level, but it’s also a symptom of a problem at another level.

The last issue is @asksol’s assertion that Pylint sucks. 🙂 When we chose our static checker, we looked at all the ones extant at the time. I won’t claim that Pylint is the epitome of checkers. But it’s a very decent checker, with great documentation, and it covers a lot of code areas. Persontally, I’d rather have a checker that’s “too picky” and I have to disable some warnings, than the inverse situation.

I like Pylint, as do many other developers. If you maintain a project and you know that checkers X, Y, and Z are in common use, you ought to be prepared that they’re all used by some of your users.