fix: grant role before creating database #5828

ltalirz · 2022-12-08T00:20:19Z

In some postgresql installations, the following fails:

template1=> create user test;
CREATE ROLE
template1=> create database test owner test;
ERROR:  must be member of role "test"

This is fixed by

template1=> grant test to azureuser;
GRANT ROLE
template1=> create database test owner test;
CREATE DATABASE

Furthermore, granting privileges to the owner is superfluous according to the postgresql docs, so we drop it.

The investigation of this issue also surfaces a bug in the error message for "manual database creation", and that the --su-db-name was not passed on from the commandline. This is fixed in passing.

In some postgresql installations, the following fails: template1=> create user test; CREATE ROLE template1=> create database test owner test; ERROR: must be member of role "test" This is fixed by template1=> grant test to azureuser; GRANT ROLE template1=> create database test owner test; CREATE DATABASE Furthermore, granting privileges to the owner is superfluous according to the postgresql docs, so we drop it. The investigation of this issue also surfaces a bug in the error message for "manual database creation", and that the `--su-db-name` was not passed on from the commandline.

ltalirz · 2022-12-08T10:14:18Z

The error is

+ verdi quicksetup --non-interactive --profile default --email aiida@localhost --first-name Giuseppe --last-name Verdi --institution Khedivial --db-backend core.psql_dos
Error: Oops! quicksetup was unable to create the AiiDA database for you.
See `verdi quicksetup -h` for how to specify non-standard parameters for the postgresql connection.
Alternatively, create the AiiDA database yourself: 
Run the following commands as a UNIX user with access to PostgreSQL (Ubuntu: $ sudo su postgres):

	$ psql template1
	==> CREATE USER "aiida_qs_aiida_477d3dfc78a2042156110cb00ae18f" WITH PASSWORD '<password>' 
	==> GRANT "aiida_qs_aiida_477d3dfc78a2042156110cb00ae3618f" TO "postgres"
	==> CREATE DATABASE "default_aiida_477d3dfc78a2042156110cb00ae3618f" OWNER "aiida_qs_aiida_477d3dfc78a2042156110cb00ae3618f" ENCODING 'UTF8' LC_COLLATE='en_US.UTF-8' LC_CTYPE='en_US.UTF-8' TEMPLATE=template0

and then use `verdi setup` instead

Traceback (most recent call last):
  File "/opt/conda/bin/verdi", line 8, in <module>
    sys.exit(verdi())
  File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 1[40](https://github.com/aiidateam/aiida-core/actions/runs/3644014214/jobs/6152813665#step:7:41)4, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/conda/lib/python3.8/site-packages/click/core.py", line 7[60](https://github.com/aiidateam/aiida-core/actions/runs/3644014214/jobs/6152813665#step:7:61), in invoke
    return __callback(*args, **kwargs)
  File "/opt/conda/lib/python3.8/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/opt/conda/lib/python3.8/site-packages/aiida/cmdline/commands/cmd_setup.py", line 185, in quicksetup
    raise exception
  File "/opt/conda/lib/python3.8/site-packages/aiida/cmdline/commands/cmd_setup.py", line 174, in quicksetup
    db_username, db_name = postgres.create_dbuser_db_safe(dbname=db_name, dbuser=db_username, dbpass=db_password)
  File "/opt/conda/lib/python3.8/site-packages/aiida/manage/external/postgres.py", line 194, in create_dbuser_db_safe
    self.create_dbuser(dbuser=dbuser, dbpass=dbpass)
  File "/opt/conda/lib/python3.8/site-packages/aiida/manage/external/postgres.py", line 112, in create_dbuser
    self.execute(_GRANT_ROLE_COMMAND.format(dbuser, self.dsn['user']))
  File "/opt/conda/lib/python3.8/site-packages/pgsu/__init__.py", line 120, in execute
    return _execute_psyco(command, dsn)
  File "/opt/conda/lib/python3.8/site-packages/pgsu/__init__.py", line 2[61](https://github.com/aiidateam/aiida-core/actions/runs/3644014214/jobs/6152813665#step:7:62), in _execute_psyco
    cursor.execute(command)
psycopg2.errors.UndefinedObject: role "None" does not exist

I.e. somehow self.dsn['user'] is None, while postgres is correctly passed along from the cli (since the "manual instructions" are correct).

When using peer authentication, the `dsn['user']` in PGSU is `None`.

ltalirz · 2022-12-08T10:46:57Z

I see - inside the container, going via the default postgres user doesn't work, and AiiDA ends up using peer authentication (which PGSU represents internally as user None).

We can use the current_user shorthand in postgresql when granting the role.

sphuber

Thanks @ltalirz some minor comments

sphuber · 2022-12-08T11:53:09Z

aiida/cmdline/commands/cmd_setup.py

@@ -177,8 +178,8 @@ def quicksetup(
                'Oops! quicksetup was unable to create the AiiDA database for you.',
                'See `verdi quicksetup -h` for how to specify non-standard parameters for the postgresql connection.\n'
                'Alternatively, create the AiiDA database yourself: ',
-                manual_setup_instructions(dbuser=su_db_username,
-                                          dbname=su_db_name), '', 'and then use `verdi setup` instead', ''
+                manual_setup_instructions(db_username=db_username,


Not concerning this line, but line 184: should we just reraise the exception? Isn't it better to simply use echo.echo_critical? Usually showing a scary traceback by default to a user from a CLI command is not a good idea.

Also just noted that the instructions above (to use verdi quicksetup -h for help) were wrong for a long time and only (coincidentally) got fixed just two weeks ago in 6469e23 😄

Sometimes the traceback can also help to see where the problem is... let me know if you want me to change it

There is a reusable option that we have --traceback that will optionally show the traceback which I think can be useful. But we can leave it for now, it is not critical

sphuber · 2022-12-08T11:56:24Z

aiida/manage/external/postgres.py

@@ -108,6 +108,8 @@ def create_dbuser(self, dbuser, dbpass, privileges=''):
            self.connection_mode == PostgresConnectionMode.PSYCOPG
        """
        self.execute(_CREATE_USER_COMMAND.format(dbuser, dbpass, privileges))
+        # this is needed on some postgresql installations


Could you please add more info on what this is doing? It seems quite weird to GRANT a user to current_user. What does that mean? Grant all rights that dbuser has also to current_user?

I've updated the comment. Let me know whether it is understandable now

aiida/manage/external/postgres.py

sphuber

Thanks @ltalirz

sphuber · 2023-01-26T12:11:37Z

~~@ltalirz did this address #5827 ? Think so right?~~

Never mind, seems it got automatically closed somehow. Didn't think it would since I didn't see the issue linked explicitly in the OP. How did Github infer that it should be closed this time? From the branch name? Surely not...

ltalirz · 2023-01-26T12:12:54Z

Yes, that is fixed now (meaning you can now use AiiDA with the hosted Azure Database for Postgresql out of the box).

ltalirz linked an issue Dec 8, 2022 that may be closed by this pull request

quicksetup: creating database with owner fails #5827

Closed

ltalirz force-pushed the fix/5827/create-with-owner branch from 3773a1d to d6f68ad Compare December 8, 2022 10:04

fix: use current_user

edc09df

When using peer authentication, the `dsn['user']` in PGSU is `None`.

ltalirz marked this pull request as ready for review December 8, 2022 10:52

ltalirz requested a review from sphuber December 8, 2022 10:52

sphuber requested changes Dec 8, 2022

View reviewed changes

ltalirz commented Jan 26, 2023

View reviewed changes

aiida/manage/external/postgres.py Outdated Show resolved Hide resolved

Update aiida/manage/external/postgres.py

080fc19

ltalirz commented Jan 26, 2023

View reviewed changes

aiida/manage/external/postgres.py Outdated Show resolved Hide resolved

ltalirz added 2 commits January 26, 2023 02:40

Update aiida/manage/external/postgres.py

e40af54

Merge branch 'main' into fix/5827/create-with-owner

97b78a1

sphuber self-requested a review January 26, 2023 12:10

sphuber approved these changes Jan 26, 2023

View reviewed changes

sphuber merged commit 76b6ba5 into aiidateam:main Jan 26, 2023

sphuber deleted the fix/5827/create-with-owner branch January 26, 2023 12:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: grant role before creating database #5828

fix: grant role before creating database #5828

ltalirz commented Dec 8, 2022 •

edited

Loading

ltalirz commented Dec 8, 2022 •

edited

Loading

ltalirz commented Dec 8, 2022

sphuber left a comment

sphuber Dec 8, 2022

sphuber Dec 8, 2022

ltalirz Jan 26, 2023

sphuber Jan 26, 2023

sphuber Dec 8, 2022

ltalirz Jan 26, 2023

sphuber Jan 26, 2023

sphuber left a comment

sphuber commented Jan 26, 2023 •

edited

Loading

ltalirz commented Jan 26, 2023 •

edited

Loading

fix: grant role before creating database #5828

fix: grant role before creating database #5828

Conversation

ltalirz commented Dec 8, 2022 • edited Loading

ltalirz commented Dec 8, 2022 • edited Loading

ltalirz commented Dec 8, 2022

sphuber left a comment

Choose a reason for hiding this comment

sphuber Dec 8, 2022

Choose a reason for hiding this comment

sphuber Dec 8, 2022

Choose a reason for hiding this comment

ltalirz Jan 26, 2023

Choose a reason for hiding this comment

sphuber Jan 26, 2023

Choose a reason for hiding this comment

sphuber Dec 8, 2022

Choose a reason for hiding this comment

ltalirz Jan 26, 2023

Choose a reason for hiding this comment

sphuber Jan 26, 2023

Choose a reason for hiding this comment

sphuber left a comment

Choose a reason for hiding this comment

sphuber commented Jan 26, 2023 • edited Loading

ltalirz commented Jan 26, 2023 • edited Loading

ltalirz commented Dec 8, 2022 •

edited

Loading

ltalirz commented Dec 8, 2022 •

edited

Loading

sphuber commented Jan 26, 2023 •

edited

Loading

ltalirz commented Jan 26, 2023 •

edited

Loading