global: change the config for celery 4 #242
Conversation
ghost
assigned 
hepcrawl/pipelines.py
Outdated
| @@ -176,7 +176,7 @@ def __init__(self): | |||
|
|
|||
| def open_spider(self, spider): | |||
| self.celery.conf.update(dict( | |||
| BROKER_URL=spider.settings['BROKER_URL'], | |||
| CELERY_BROKER_URL=spider.settings['CELERY_BROKER_URL'], | |||
There was a problem hiding this comment.
does this require reverting https://gitlab.cern.ch/ai/it-puppet-hostgroup-inspire/commit/bb030ed9dea6a231cdedde4d8aa1349f92e91425 ?
There was a problem hiding this comment.
Eventually yes, although I will also need to fix the typo I made in https://gitlab.cern.ch/ai/it-puppet-hostgroup-inspire/commit/bb030ed9dea6a231cdedde4d8aa1349f92e91425#ece1b7b92ff70b0371eddaad468a2895e0fc8bb7_260_260.
Signed-off-by: Jacopo Notarstefano <jacopo.notarstefano@gmail.com>
Signed-off-by: Jacopo Notarstefano <jacopo.notarstefano@gmail.com>
| BROKER_URL = "amqp://guest:guest@rabbitmq:5672//" | ||
| CELERY_ALWAYS_EAGER = True | ||
| CELERY_CACHE_BACKEND = 'memory' | ||
| CELERY_EAGER_PROPAGATES_EXCEPTIONS = True |
There was a problem hiding this comment.
These three configuration variables have not had effect for a long time, as they are not valid or ignored in Celery 4, so we can safely remove them.
| @@ -19,11 +19,8 @@ | |||
|
|
|||
|
|
|||
| class Config(object): | |||
| CELERY_RESULT_BACKEND = "amqp://guest:guest@rabbitmq:5672//" | |||
| BROKER_URL = "amqp://guest:guest@rabbitmq:5672//" | |||
| CELERY_ALWAYS_EAGER = True | |||
There was a problem hiding this comment.
I'm surprised the tests don't require some flavor of this, but they seem to pass, so it must be OK.
There was a problem hiding this comment.
This happens to work because of the Celery Monitor, which essentially attaches receivers that listen to signals like "a task completed successfully":
hepcrawl/hepcrawl/testlib/celery_monitor.py
Lines 76 to 82 in b753c22
BTW, as remarked in #241, hepcrawl has been tested for a long time against Celery 4, and the name of this parameter changed to task_always_eager, so this configuration option has had no effect for who knows how much time.
|
@ammirate Don't know how this affects your e2e tests |
Description:
#241 forgot to update the configuration for Celery 4, which led to a failure to perform the harvest on QA and https://gitlab.cern.ch/ai/it-puppet-hostgroup-inspire/commit/bb030ed9dea6a231cdedde4d8aa1349f92e91425 to fix it.
This PR aims to make the above commit unnecessary.I wasn't able to useCELERY_BROKER_URLinstead ofBROKER_URL. I don't know the reason, but all this means is that the above commit is here to stay.Checklist:
RFCand look for it).