update to be compatible with newest version of celery

[dconathan]

A recent version of celery broke our implementation, hence celery==3.1.25. It's possibly just a refactoring of some method names, but it could be more.

[stsievert]

Celery has a guide on going from 3.1 to 4.0: http://docs.celeryproject.org/en/latest/whatsnew-4.0.html#upgrading-from-celery-3-1

[dconathan]

I figured it out. They changed how the prefetch multiplier option is set. It's an easy fix. I'll submit a PR to fix it soon.

They are also switching all the upper case constants/settings to lower case... (backwards compatible for now but are recommending switching "as soon as possible". This will involve some more substantial (but hopefully harmless) changes to constants/launch scripts/docker-compose files...

I'll test this out tomorrow...

[lalitkumarj]

Please keep us in the loop on this. It could have large ramifications for some current experiments.

…

On Jan 24, 2017 20:34, "dconathan" ***@***.***> wrote: I figured it out. They changed how the prefetch multiplier option is set. It's an easy fix. I'll submit a PR to fix it soon. They are also switching all the upper case constants/settings to lower case... (backwards compatible for now but are recommending switching "as soon as possible". This will involve some more substantial (but hopefully harmless) changes to constants/launch scripts/docker-compose files... I'll test this out tomorrow... — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#159 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABWhGA7bvfYBdyBs581tbM42Nuz022plks5rVtC7gaJpZM4LOplV> .

[dconathan]

Okay. Like I said, there are two changes here:

1. To fix compatibility with 4.0, we need to make very minor changes to next_worker_startup.sh
2. To maintain compatibility with future versions, refactor the constants as lower-case. I haven't looked into the exact scope, but this would change at least a few files, and would require some testing to make sure celery is picking up our options and not just using its own defaults somewhere.

However, I just tested and making the 1st change DOES break 3.1.25 (presumably the 2nd does too), so either of these changes will require updating celery/rebuilding the containers.*

So... do we want to hold off on this upgrade and incorporate it into the next major NEXT release?

*Containers can be rebuilt from cache by making the change in the requirements.txt file and just running docker-compose build ... it only takes a few seconds... something to keep in mind.

[dconathan]

I've got a PR ready for this. I'm going to wait until some of the existing PRs get merged so things don't get crazy. Details will come but I made it so it's backwards compatible so we can roll back if necessary. I'm not updating constants yet because that breaks too many things with 3.1.25.

[erinzm]

@dconathan We're contemplating a release soon-ish; can you open a PR with that code?

[erinzm]

Backed off on this change for now.
Some comments @dconathan made on Slack about why we're not doing this:

I have moved to celery 4. I have noticed getQuery has been pushing .5-1s delays for no apparent reason and I haven't gotten around to debugging it... this could explain that.

Tomorrow I'll rebuild with celery 3.1.25 and see if that fixes it...
Just reverted back to celery 3.1.25 and getQuery delays are back down to sub .1s range… so almost certainly seeing a slowdown due to celery 4

I was wondering about it. Almost all the extra time was “enqueued” time according to the charts.
...
I guess there’s one reason not to switch to the latest and greatest

[erinzm]

@dconathan can you test this again, since it's apparently fixed (c.f. celery/celery#3814)