db.pytest_plugin: Make pytest-postgresql an optional runtime dependency
Currently, this pulls the postgresql installation when installing swh.core which is not something wanted. That bypasses the current production pinning done in puppet and installs the latest postgresql-13 for example.
That particular dependency should be installed alongside swh modules which needs the core db pytest-plugin for their tests.
As impacts, i expected current packages needing this to break a bit but those already declares the pytest-postgresql as test requirements (i probably forgot to remove them). So, nothing to do, their builds will stay green:
-
swh.scheduler -
swh.storage -
swh.vault
Related to swh/infra/sysadm-environment#2746 (closed)
Test Plan
tox
Migrated from D4376 (view on Phabricator)
Merge request reports
Activity
Build is green
Patch application report for D4376 (id=15480)
Could not rebase; Attempt merge onto eefe1b63...
Updating eefe1b6..016ad26 Fast-forward requirements-db.txt | 2 +- requirements-test-db.txt | 2 +- swh/core/cli/db.py | 54 +++++++++++++++++++++++++++++++++++++------ swh/core/db/pytest_plugin.py | 10 ++++++-- swh/core/db/tests/test_cli.py | 6 +++-- 5 files changed, 61 insertions(+), 13 deletions(-)Changes applied before test
commit 016ad26170fa86bca6a32dd9dc14812d5ef42d05 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Oct 29 09:20:32 2020 +0100 db.pytest_plugin: Make the pytest-postgresql an optional runtime dependency Currently, this pulls the postgresql installation when installing swh.core which is not something wanted. That bypasses the current production pinning done in puppet and installs the latest postgresql-13 for example. That particular dependency should be installed alongside swh modules which needs the core db pytest-plugin for their tests. commit 745448399ebda14f08a9286b08fb6dd008d9504b Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Wed Oct 28 18:27:46 2020 +0100 cli.db: Open init-admin subcmd to initialize superuser-level scripts In staging, the db is already created by puppet, the current create database stanza forced within the create-db cannot work. Ths subsequent steps of postgresql extensions installation is required though. This will allow it. Related to swh/infra/sysadm-environment#2736See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/122/ for more details.
-
Build is green
Patch application report for D4376 (id=15481)
Could not rebase; Attempt merge onto eefe1b63...
Updating eefe1b6..272f91b Fast-forward requirements-db.txt | 2 +- requirements-test-db.txt | 2 +- swh/core/cli/db.py | 54 +++++++++++++++++++++++++++++++++++++------ swh/core/db/pytest_plugin.py | 10 ++++++-- swh/core/db/tests/test_cli.py | 6 +++-- 5 files changed, 61 insertions(+), 13 deletions(-)Changes applied before test
commit 272f91b939af9846dbc9c26172b1ead36885e156 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Oct 29 09:20:32 2020 +0100 db.pytest_plugin: Make pytest-postgresql an optional runtime dependency Currently, this pulls the postgresql installation when installing swh.core which is not something wanted. That bypasses the current production pinning done in puppet and installs the latest postgresql-13 for example (even on node without postgresql). That particular dependency should be installed alongside swh modules which needs the core db pytest-plugin for their tests. commit 745448399ebda14f08a9286b08fb6dd008d9504b Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Wed Oct 28 18:27:46 2020 +0100 cli.db: Open init-admin subcmd to initialize superuser-level scripts In staging, the db is already created by puppet, the current create database stanza forced within the create-db cannot work. Ths subsequent steps of postgresql extensions installation is required though. This will allow it. Related to swh/infra/sysadm-environment#2736See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/123/ for more details.
Where is the requirements-test-db.txt file used?
Having to declare transitive dependencies in modules using this pytest plugin sounds quite wrong. And having an extra version of postgresql installed isn't an issue at all, the postgresql Debian packages have been designed to be coinstallable without issues.
Having to declare transitive dependencies in modules using this pytest plugin sounds quite wrong.
Yes, I am not entirely satisfied with this. But the pb described in the description is real and I don't know how to prevent it more properly than this...
And having an extra version of postgresql installed isn't an issue at all, the postgresql Debian packages have been designed to be coinstallable without issues.
It created actual issues on db (staging/vagrant) servers (which currently pins the 12 versions). A 13 (new default version) one got installed unexpectedly (well for me ¯_(ツ)_/¯).
Somehow both servers ran on the same port which did not bode well. Or sometimes, one did not start or the wrong one did... or some combination of badness.
Also, without this, this means than any node installing a swh module requiring swh.core (all?) will install postgresql. In my mind, that's not a desired behavior.
-
! In !183 (closed), @ardumont wrote: Having to declare transitive dependencies in modules using this pytest plugin sounds quite wrong.
Yes, I am not entirely satisfied with this. But the pb described in the description is real and I don't know how to prevent it more properly than this...
And having an extra version of postgresql installed isn't an issue at all, the postgresql Debian packages have been designed to be coinstallable without issues.
It created actual issues on db (staging/vagrant) servers (which currently pins the 12 versions). A 13 (new default version) one got installed unexpectedly (well for me ¯_(ツ)_/¯).
Somehow both servers ran on the same port which did not bode well. Or sometimes, one did not start or the wrong one did... or some combination of badness.
These issues need to be fixed on the puppet side anyway. If this change lands, nothing will prevent another dependency from pulling the postgresql-13 package and wreaking havoc. I guess the postgresql server puppet profile should just ship a
/etc/postgresql-common/createcluster.conffile which would avoid the postgres package install creating the default postgres clusters (and order the creation of that file before any package install).Also, without this, this means than any node installing a swh module requiring swh.core (all?) will install postgresql. In my mind, that's not a desired behavior.
If that's really a problem, then we should separate the swh.core pytest plugin in a separate Debian package that would only be installed as a build-dependency of the modules that need it for their tests.
-
These issues need to be fixed on the puppet side anyway. If this change lands, nothing will prevent another dependency from pulling the postgresql-13 package and wreaking havoc
Indeed. But it will prevent today to deploy massively postgresql everywhere it's not required. So that's a first step.
I guess the postgresql server puppet profile should just ship a /etc/postgresql-common/createcluster.conf file which would avoid the postgres package install creating the default postgres clusters (and order the creation of that file before any package install).
(iiuc what you suggest) That will just prevent that misbehavior on db nodes only... So ok, this is needed for the db nodes.
Nonetheless, that's not enough for the other nodes...
Or do you want to add this file to every node? That would be surprising to have a /etc/postgresql-common/... configuration directory on nodes without postgresql... So i'm a bit dubious...
If that's really a problem,
Installing postgres where it's not required is a problem to me. If it's not required, it's not to be installed.
then we should separate the swh.core pytest plugin in a separate Debian package that would only be installed as a build-dependency of the modules that need it for their tests.
ok, i think i understand. And then, python-side, we can add thed correct dependency in requirements-test.txt from the swh modules that need it (scheduler, ...) And debian side, as you said, update the debian/control (swh.core) with that new package and for those swh modules (scheduler...), add in their debian/control build-dependency that very swh.core.pytest (or something) debian package.
-
superseded by !186 (closed) !190 (closed)
mentioned in merge request !186 (closed)