Project

Profile

Help

Refactor #460

Use celery.signals.worker_process_init to start the database connection

Added by rbarlow almost 6 years ago. Updated over 1 year ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
2.8.0
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
Pulp 2
Sprint:
April 2015
Quarter:

Description

We attempted to do this in 2.4.0, but we ran into a few different issues. One of them was that for some reason our Celery tasks weren't being discovered by the workers. It was decided that this was too large of a change to go in so late in the 2.4.0 release cycle, and that we would do it the right way in a future release (perhaps 2.5.0, or 3.0.0).

Here is the pull request that first introduced the change that we would like to reintroduce in the future:

https://github.com/pulp/pulp/pull/1019

That change introduced two issues:

1) The qpid monkey patching no longer happened early enough in the worker startup, and so kombu would not work. I fixed this by moving the qpid import in initialization.py to be the first import, which revealed a second problem:

2) The Celery workers no longer discovered our tasks for some reason. I never learned why this happened before we decided to back out the change and revisit it later.

Here is the PR that removed the change:

https://github.com/pulp/pulp/pull/1023

+ This bug was cloned from Bugzilla Bug #1113192 +

Associated revisions

Revision d17e9e5c View on GitHub
Added by rbarlow over 5 years ago

Ensure that tests share a single DB conenction.

Many of our tests independently connected to the database, and they did not always ensure that they were connecting to the test database. This commit fixes the tests so they all use the test database.

Additionally, this commit refactors all of the entry points such that the connection is no longer started as a side effect of importing a module. Most of our code already worked this way, but this commit refactors the Celery workers to start the database connection using a signal instead of through an import.

This commit also makes it an error for the connection to be initialized more than once.

https://pulp.plan.io/issues/131 https://pulp.plan.io/issues/460

fixes #131 fixes #460

Revision d17e9e5c View on GitHub
Added by rbarlow over 5 years ago

Ensure that tests share a single DB conenction.

Many of our tests independently connected to the database, and they did not always ensure that they were connecting to the test database. This commit fixes the tests so they all use the test database.

Additionally, this commit refactors all of the entry points such that the connection is no longer started as a side effect of importing a module. Most of our code already worked this way, but this commit refactors the Celery workers to start the database connection using a signal instead of through an import.

This commit also makes it an error for the connection to be initialized more than once.

https://pulp.plan.io/issues/131 https://pulp.plan.io/issues/460

fixes #131 fixes #460

History

#1 Updated by rbarlow over 5 years ago

  • Tracker changed from Task to Refactor
  • Status changed from NEW to ASSIGNED
  • Assignee set to rbarlow
  • Sprint/Milestone set to 15
  • Platform Release set to master
  • Tags Sprint Candidate added

#2 Updated by bmbouter over 5 years ago

  • Sprint Candidate set to Yes
  • Tags deleted (Sprint Candidate)

#3 Updated by rbarlow over 5 years ago

  • Status changed from ASSIGNED to POST
  • Groomed set to No

#4 Updated by rbarlow over 5 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#5 Updated by mhrivnak over 5 years ago

  • Groomed changed from No to Yes

#6 Updated by rbarlow about 5 years ago

  • Platform Release changed from master to 2.8.0

#7 Updated by dkliban@redhat.com almost 5 years ago

  • Status changed from MODIFIED to 5

#8 Updated by dkliban@redhat.com over 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#9 Updated by bmbouter over 2 years ago

  • Sprint set to April 2015

#10 Updated by bmbouter over 2 years ago

  • Sprint/Milestone deleted (15)

#11 Updated by bmbouter over 1 year ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF