db: Grant read access to guest user on all tables of the schema
This now automatically grant read-only access to the guest user at db initialization or upgrade time. This way, sysadm no longer have to manually alter the schema after initialization or upgrade.
Related to swh/infra/sysadm-environment#4228 (closed)
Test Plan
tox should still be happy
Migrated from D7913 (view on Phabricator)
Merge request reports
Activity
Build has FAILED
Patch application report for D7913 (id=28524)
Rebasing onto 749ac571...
Current branch diff-target is up to date.Changes applied before test
commit 0c2ff429ef9b5b5d2a5bab4e92944227a6b531fb Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com> Date: Tue May 31 09:18:03 2022 +0200 db: Grant read access to guest user on all tables of the schema Whether at db initialization or upgrade time. Related to swh/infra/sysadm-environment#4228Link to build: https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/434/ See console output for more information: https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/434/console
-
Build is green
Patch application report for D7913 (id=28525)
Rebasing onto 749ac571...
Current branch diff-target is up to date.Changes applied before test
commit a5918c1b25ce8819172683ec841bfeab73efe564 Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com> Date: Tue May 31 09:18:03 2022 +0200 db: Grant read access to guest user on all tables of the schema Whether at db initialization or upgrade time. Related to swh/infra/sysadm-environment#4228See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/435/ for more details.
Tests for the cli are using again the pytest-plugin db tool (i think it got removed at some point for some reason).
I believe this comes from f8a07dfa0bf6
The question being, is it required to use this version of the postgresql factory instead of the stock one? I was hoping to rather get rid of this custom DbJanitor back then: it generates more complexity than it saves time. And now that we support recent-ish versions of pytest-postgresql, we do benefit from the "test db creation from template" feature, making the startup time for each test pretty similar than using this custom janitor.
With @douardda, we agreed on irc to postpone this diff a bit the time david found back how to properly update the postgres template (to integrate the guest user). So the part spawning back the pytest-plugin postgresql_fact becomes unnecessary [1]
- [1] That's getting slowly retired from other swh modules.
-
@douardda Any news on how to modify a db template for the tests?
-
@douardda Any news on how to modify a db template for the tests?
15:32 douardda ben normalement, l'argument load du postgresql_proc() va etre utilisé pour creer le template de base utilisé ensuite pour creer la base pour chaque test 15:33 douardda c'est ce que j'utilise dans https://forge.softwareheritage.org/source/swh-storage/browse/master/swh/storage/pytest_plugin.py$20-28 par ex 15:34 douardda la le 'load' va de facto executer la mecanique swh.core d'init d'une base 15:34 ardumont ok dc je devrais pouvoir utiliser ca et ajouter une fonction qui ajoute un user guest ds la liste passee a 'load' 15:35 douardda tu peux ajouter un chemin de script sql directement comme element dans cette list -
Build is green
Patch application report for D7913 (id=28683)
Rebasing onto e1a1d84e...
Current branch diff-target is up to date.Changes applied before test
commit cc5e2957e739e31b2b448f5fb90a97af415ace35 Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com> Date: Tue May 31 09:18:03 2022 +0200 db: Grant read access to guest user on all schema tables This now automatically grant read-only access to the guest user at db initialization or upgrade time. This way, sysadm no longer have to manually alter the schema after initialization or upgrade. Related to swh/infra/sysadm-environment#4228See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/444/ for more details.
-
One remark from @vsellier got my attention, it will break the docker setup. Maybe @douardda hinted at it as well...
He is not wrong there, see for example [1]. So some adaptations will be needed there.
- [1]
swh-scheduler_1 | swh-scheduler database setup swh-scheduler_1 | Creating extensions... swh-scheduler_1 | psql:/srv/softwareheritage/venv/lib/python3.7/site-packages/swh/scheduler/sql/10-superuser-init.sql:1: NOTICE: extension "uuid-ossp" already exists, skipping swh-scheduler_1 | Traceback (most recent call last): swh-scheduler_1 | File "/srv/softwareheritage/venv/bin/swh", line 33, in <module> swh-scheduler_1 | sys.exit(load_entry_point('swh.core', 'console_scripts', 'swh')()) swh-scheduler_1 | File "/src/swh-core/swh/core/cli/__init__.py", line 184, in main swh-scheduler_1 | return swh(auto_envvar_prefix="SWH") swh-scheduler_1 | File "/srv/softwareheritage/venv/lib/python3.7/site-packages/click/core.py", line 1137, in __call__ swh-scheduler_1 | return self.main(*args, **kwargs) swh-scheduler_1 | File "/srv/softwareheritage/venv/lib/python3.7/site-packages/click/core.py", line 1062, in main swh-scheduler_1 | rv = self.invoke(ctx) swh-scheduler_1 | File "/srv/softwareheritage/venv/lib/python3.7/site-packages/click/core.py", line 1668, in invoke swh-scheduler_1 | return _process_result(sub_ctx.command.invoke(sub_ctx)) swh-scheduler_1 | File "/srv/softwareheritage/venv/lib/python3.7/site-packages/click/core.py", line 1668, in invoke swh-scheduler_1 | return _process_result(sub_ctx.command.invoke(sub_ctx)) swh-scheduler_1 | File "/srv/softwareheritage/venv/lib/python3.7/site-packages/click/core.py", line 1404, in invoke swh-scheduler_1 | return ctx.invoke(self.callback, **ctx.params) swh-scheduler_1 | File "/srv/softwareheritage/venv/lib/python3.7/site-packages/click/core.py", line 763, in invoke swh-scheduler_1 | return __callback(*args, **kwargs) swh-scheduler_1 | File "/src/swh-core/swh/core/cli/db.py", line 123, in db_init_admin swh-scheduler_1 | init_admin_extensions(module, dbname) swh-scheduler_1 | File "/src/swh-core/swh/core/db/db_utils.py", line 607, in init_admin_extensions swh-scheduler_1 | execute_sqlfiles(sqlfiles, conninfo) swh-scheduler_1 | File "/src/swh-core/swh/core/db/db_utils.py", line 700, in execute_sqlfiles swh-scheduler_1 | c.execute(query) swh-scheduler_1 | psycopg2.errors.UndefinedObject: role "guest" does not exist swh-scheduler_1 | -
One remark from @vsellier got my attention, it will break the docker setup. Maybe @douardda hinted at it as well...
He is not wrong there, see for example [1]. So some adaptations will be needed there.
[1]
Taken care of with swh-environment!196 (closed).
mentioned in merge request swh-environment!196 (closed)
-
maybe only show a warning if the grant query fails (rather than crashing)?
good point, i'll check how to properly do that.
@vsellier suggested to check if the guest user exist but i guess all this are implementation detail.
What we want is indeed not to crash if this grant does not work for whatever reasons.
Thanks.
Some references in the commit message have been migrated:
- T4228 is now swh/infra/sysadm-environment#4228 (closed)
-
Build is green
Patch application report for D7913 (id=28709)
Rebasing onto e1a1d84e...
Current branch diff-target is up to date.Changes applied before test
commit 47d9e8e22fa6c88eadbd552eebfbc42e51f72813 Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com> Date: Tue May 31 09:18:03 2022 +0200 db: Grant read access to guest user on all schema tables This now automatically grant read-only access to the guest user at db initialization or upgrade time. This way, sysadm no longer have to manually alter the schema after initialization or upgrade. Related to swh/infra/sysadm-environment#4228See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/445/ for more details.
