|
Date: Fri, 23 Jan 2015 16:55:25 +0100
|
|
From: rbarlow@redhat.com
|
|
To: dropbox+pulp+c71e@plan.io
|
|
Message-ID: <54c26eedbcc37_2a953fc034d53ea8964ac@apollo.mail>
|
|
in-reply-to: redmine.issue-131.20150122203107@plan.io
|
|
Subject: Re: [Pulp - Refactor #131] (NEW) Move
|
|
pulp.server.db.connection.initialize() calls exclusively to entry points
|
|
Mime-Version: 1.0
|
|
Content-Type: multipart/mixed;
|
|
boundary=hgOlGBOJc6S1wK0pfsHTQ1whiEGP43Oua;
|
|
charset=UTF-8
|
|
Content-Transfer-Encoding: 7bit
|
|
X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24
|
|
X-HE-Spam-Level: -----
|
|
X-HE-Spam-Score: -5.0
|
|
X-HE-Spam-Report: Content analysis details: (-5.0 points) pts rule name
|
|
description ---- ----------------------
|
|
-------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL:
|
|
Sender listed at http://www.dnswl.org/, high trust [209.132.183.28 listed in
|
|
list.dnswl.org]
|
|
X-HE-SPF: PASSED
|
|
|
|
|
|
--hgOlGBOJc6S1wK0pfsHTQ1whiEGP43Oua
|
|
Content-Type: text/plain;
|
|
charset=utf-8
|
|
Content-Transfer-Encoding: quoted-printable
|
|
|
|
On 01/22/2015 03:31 PM, Brian Bouterse wrote:
|
|
> * The test runner used by platform and all plugins
|
|
> pulp.devel.test_runner.run_tests
|
|
>
|
|
<https://github.com/pulp/pulp/blob/master/devel/pulp/devel/test_runner.py=
|
|
#L13>
|
|
|
|
Since the test runner is not the same process as the tests, it cannot
|
|
make the call to initialize().
|
|
> --- Please write your response above this line ---
|
|
> =
|
|
|
|
> Issue #131 has been reported by Brian Bouterse.
|
|
> -----------------------------------------------------------------------=
|
|
-
|
|
> =
|
|
|
|
> =
|
|
|
|
> Refactor #131: Move pulp.server.db.connection.initialize() calls
|
|
> exclusively to entry points <https://pulp.plan.io/issues/131>
|
|
> =
|
|
|
|
> * Author: Brian Bouterse
|
|
> * Status: NEW
|
|
> * Priority: Normal
|
|
> * Assignee:
|
|
> * Category:
|
|
> * Sprint/Milestone:
|
|
> * Target Release: =
|
|
|
|
> =
|
|
|
|
> *The Problem*
|
|
> Throughout the production and test code for Pulp and its plugins there
|
|
> are many calls to pulp.server.db.connection.initialize(). Starting with=
|
|
|
|
> Pulp 2.6.0, mongoengine caches the last initialized connection as the
|
|
> connection all database interaction will use. At test run-time can caus=
|
|
e
|
|
> the testing database to mistakenly attach to the production database
|
|
> through liberal calls to connection.initialize() that occur without the=
|
|
|
|
> server.conf being mocked. This causes horrible things to occur to Pulp
|
|
> development environments. Also, its sloppy to not have a structured way=
|
|
|
|
> to connect to the database.
|
|
> =
|
|
|
|
> This also leads to a second problem which is that many of these calls t=
|
|
o
|
|
> pulp.server.db.connection.initialize() are at the module level which
|
|
> means that simply by importing that code you are connecting to the
|
|
> database! A very unexpected side-effect that makes the above
|
|
> test-environment database switching occur depending on what you import.=
|
|
|
|
> Ouch!
|
|
> =
|
|
|
|
> *The goal*
|
|
> Move all of the pulp.server.db.connection.initialize() calls to the
|
|
> required entry point for all Pulp components to run. Each component tha=
|
|
t
|
|
> starts shall make exactly one call to
|
|
> pulp.server.db.connection.initialize() as part of the entry startup.
|
|
> =
|
|
|
|
> *The Entry Points*
|
|
> We need to consider entry points for both the development and productio=
|
|
n
|
|
> uses where Pulp code will be run that need the database connection to
|
|
> already be initialized.
|
|
> =
|
|
|
|
> * The pulp-server WSGI app. /srv/pulp/webservices.wsgi
|
|
> <https://github.com/pulp/pulp/blob/master/server/srv/pulp/webservic=
|
|
es.wsgi>
|
|
> * The pulp_celerybeat entry point
|
|
> pulp.server.async.scheduler.Scheduler
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/async/=
|
|
scheduler.py#L241>
|
|
> * The pulp_workers entry point. This is shared by both pulp_workers
|
|
> and pulp_resource_manager. pulp.server.async.app
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/async/=
|
|
app.py>
|
|
> * The repo protection WSGI entry point. /srv/pulp/repo_auth.wsgi
|
|
> <https://github.com/pulp/pulp_rpm/blob/master/plugins/srv/pulp/repo=
|
|
_auth.wsgi>
|
|
> * The pulp_puppet forge pre 33 API WSGI app.
|
|
> /srv/pulp/puppet_forge_pre33_api.wsgi
|
|
> <https://github.com/pulp/pulp_puppet/blob/master/pulp_puppet_plugin=
|
|
s/srv/pulp/puppet_forge_pre33_api.wsgi>
|
|
> * The pulp_puppet forge post 33 API WSGI app.
|
|
> /srv/pulp/puppet_forge_post33_api.wsgi
|
|
> <https://github.com/pulp/pulp_puppet/blob/master/pulp_puppet_plugin=
|
|
s/srv/pulp/puppet_forge_post33_api.wsgi>
|
|
> * The test runner used by platform and all plugins
|
|
> pulp.devel.test_runner.run_tests
|
|
> <https://github.com/pulp/pulp/blob/master/devel/pulp/devel/test_run=
|
|
ner.py#L13>
|
|
> =
|
|
|
|
> *Steps*
|
|
> 1. Each one of the entry point components above need to have exactly on=
|
|
e
|
|
> call to pulp.server.db.connection.initialize()
|
|
> =
|
|
|
|
> 2. All other calls to pulp.server.db.connection.initialize() needs to b=
|
|
e
|
|
> removed. This includes all platform and all plugins. Search and destroy=
|
|
!
|
|
> =
|
|
|
|
> 3. pulp.server.db.connection.initialize()
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/db/connect=
|
|
ion.py#L30>
|
|
> needs to have a behavior added that raises an Exception if it has
|
|
> already been called. This should enforce that a second call to
|
|
> initialize() will fail hard. Maybe have the exception live in the
|
|
> db.connection similar to PulpDatabaseFailure from step (3).
|
|
> =
|
|
|
|
> 3. Rework pulp.server.db.connection.PulpCollectionFailure to be
|
|
> PulpDatabaseFailure instead.
|
|
> =
|
|
|
|
> 4. Add a behavior to get_collection()
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/db/connect=
|
|
ion.py#L229>
|
|
> get_database()
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/db/connect=
|
|
ion.py#L242>
|
|
> and get_connection()
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/db/connect=
|
|
ion.py#L250>
|
|
> whereby if the database has not been initialized successfully a
|
|
> PulpDatabaseFailure (step 3) is raised. This will ensure that when the
|
|
> database is to be used initialize has been called at least 1 time.
|
|
> =
|
|
|
|
> 5. Add test coverage for changes 2-4.
|
|
> =
|
|
|
|
> *Testing Concerns*
|
|
> Any testing code should rely on the testing entry point to provide the
|
|
> call to connection.initialize(). If the test runner provides the call t=
|
|
o
|
|
> initialize() then an exception will be raised on the second call to
|
|
> connection.initialize().
|
|
> =
|
|
|
|
> * The WSGI scripts are so simple they likely don't need to be tested
|
|
> at all.
|
|
> * The celery worker entry point is also very simple and can likely no=
|
|
t
|
|
> be tested.
|
|
> * The celerybeat entry point can be tested because it is guarded by
|
|
> scheduler.setup_schedules
|
|
> <https://github.com/pulp/pulp/blob/master/server/pulp/server/async/=
|
|
scheduler.py#L333>
|
|
> method. We can mock the connection.initialize() before we call
|
|
> setup_schedules().
|
|
> =
|
|
|
|
> -----------------------------------------------------------------------=
|
|
-
|
|
> =
|
|
|
|
> You have received this notification because you have either subscribed
|
|
> to or are involved in a project on pulp Planio.
|
|
> To change your notification preferences, please click here:
|
|
> https://pulp.plan.io/my/account
|
|
> =
|
|
|
|
> =
|
|
|
|
> =
|
|
|
|
> This notification was cheerfully delivered by <https://plan.io/>
|
|
> =
|
|
|
|
> Planio <https://plan.io/>
|
|
> =
|
|
|
|
|
|
|
|
--hgOlGBOJc6S1wK0pfsHTQ1whiEGP43Oua--
|
.