Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update celery requirements to its latest version #6448

Merged
merged 2 commits into from Jan 9, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Dec 8, 2019

Upgrading celery requires to upgrade their dependencies as
well (kombu, redis). By using their latest version, it could help with
the random disconnection of the workers that we suffer sometimes.

After upgrading them all, the errors we have found when we pinned the
versions they all disappeared.

Closes #4643

Upgrading celery requires to upgrade their dependencies as
well (kombu, redis). By using their latest version, it could help with
the random disconnection of the workers that we suffer sometimes.

After upgrading them all, the errors we have found when we pinned the
versions they all disappeared.
@humitos humitos requested a review from Dec 8, 2019
@stsewd
Copy link
Member

@stsewd stsewd commented Dec 9, 2019

Did you test with ALWAYS_EAGER = True? Core team should be using it to False, but new contributors will hit an error.

Also, this will closes #4643

@humitos
Copy link
Member Author

@humitos humitos commented Dec 9, 2019

Did you test with ALWAYS_EAGER = True?

Yes. All tests are ran with ALWAYS_EAGER = True.

@stsewd
Copy link
Member

@stsewd stsewd commented Dec 9, 2019

Yes. All tests are ran with ALWAYS_EAGER = True.

The error happens when triggering a build, I don't think it was detected inside the tests.

@humitos
Copy link
Member Author

@humitos humitos commented Dec 9, 2019

Confirm. Triggering a build does work.

stsewd
stsewd approved these changes Dec 9, 2019
@stsewd
Copy link
Member

@stsewd stsewd commented Dec 9, 2019

So, the issue on celery is still open celery/celery#5330

But I think we don't use this anymore

def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable=redefined-builtin

That's why we don't hit this issue anymore.

Copy link
Member

@ericholscher ericholscher left a comment

I'm thinking we might still hit this in production, but I'm 👍 on trying to upgrade it, because falling super far behind on versions isn't good.

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Dec 17, 2019

Let's merge this after our deploy today, so we have time to test it out in dev across different environments before we ship it to prod over the holidays.

@humitos humitos added the Status: blocked label Dec 17, 2019
@humitos humitos removed the Status: blocked label Jan 9, 2020
@humitos humitos merged commit 7f32949 into master Jan 9, 2020
3 checks passed
@humitos humitos deleted the humitos/update-requirements branch Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants