From 7fd221da85a36227ae465f166174e3c72683842f Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Tue, 5 Nov 2024 10:49:32 +0100
Subject: [PATCH] cli/db: show the actual error that occurred during sql script
execution, if any
When executing a `swh db init` command, the executed sql files for the
package may fail, but the 'swh db' tool would eat the error reported by
sqlsh. Make these errors shown as output of the process if such an error
occurred.
e.g.:
$ swh db init test:fail
Error during database setup
Command '['psql', '--quiet', '--no-psqlrc', '-v', 'ON_ERROR_STOP=1', '-d', 'postgresql://postgres@127.0.0.1:24370/tests', '-f', '/venv/swh-environment/swh-core/swh/core/db/tests/data/test/fail/sql/40-funcs.sql']' returned non-zero exit status 3.
Process output:
psql:/venv/swh-environment/swh-core/swh/core/db/tests/data/test/fail/sql/40-funcs.sql:6: ERROR: function public.digest(text, unknown) does not exist
LINE 5: select encode(public.digest($1, 'sha1'), 'hex')
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
Aborted!
---
swh/core/cli/db.py | 15 +++++++--
swh/core/db/tests/data/README | 3 ++
swh/core/db/tests/data/test/fail/__init__.py | 0
.../db/tests/data/test/fail/sql/30-schema.sql | 6 ++++
.../db/tests/data/test/fail/sql/40-funcs.sql | 6 ++++
swh/core/db/tests/test_cli.py | 33 +++++++++++++++----
6 files changed, 53 insertions(+), 10 deletions(-)
mode change 100755 => 100644 swh/core/cli/db.py
create mode 100644 swh/core/db/tests/data/test/fail/__init__.py
create mode 100644 swh/core/db/tests/data/test/fail/sql/30-schema.sql
create mode 100644 swh/core/db/tests/data/test/fail/sql/40-funcs.sql
diff --git a/swh/core/cli/db.py b/swh/core/cli/db.py
old mode 100755
new mode 100644
index 16c7e822..7b9d5ce1
--- a/swh/core/cli/db.py
+++ b/swh/core/cli/db.py
@@ -6,6 +6,7 @@
import logging
from os import environ
+from subprocess import CalledProcessError
from typing import Any, Dict, List, Optional, Tuple
import warnings
@@ -340,9 +341,17 @@ def initialize_one(package, module, backend_class, flavor, dbname, cfg):
logger.debug("db_init %s flavor=%s dbname=%s", module, flavor, dbname)
dbmodule = f"{package}:{cfg['cls']}"
- initialized, dbversion, dbflavor = populate_database_for_package(
- dbmodule, dbname, flavor
- )
+ try:
+ initialized, dbversion, dbflavor = populate_database_for_package(
+ dbmodule, dbname, flavor
+ )
+ except CalledProcessError as exc:
+ click.secho("Error during database setup", fg="red", bold=True)
+ click.echo(str(exc))
+ click.echo("Process output:")
+ click.echo(exc.stderr)
+ raise click.Abort()
+
if dbversion is not None:
click.secho(
"ERROR: the database version has been populated by sql init scripts. "
diff --git a/swh/core/db/tests/data/README b/swh/core/db/tests/data/README
index 27ce3810..1694a355 100644
--- a/swh/core/db/tests/data/README
+++ b/swh/core/db/tests/data/README
@@ -23,3 +23,6 @@ test:
will define 2 'swh.core.db' powered backends initialized from (resp.)
'./postgresql/sql' and './cli2/sql'.
+
+The 'test:fail' backend is expected to fail when trying to initialize the
+database (missing the pgcrypto extension, so the psql execution should fail).
diff --git a/swh/core/db/tests/data/test/fail/__init__.py b/swh/core/db/tests/data/test/fail/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/swh/core/db/tests/data/test/fail/sql/30-schema.sql b/swh/core/db/tests/data/test/fail/sql/30-schema.sql
new file mode 100644
index 00000000..64289f78
--- /dev/null
+++ b/swh/core/db/tests/data/test/fail/sql/30-schema.sql
@@ -0,0 +1,6 @@
+-- origin table
+create table if not exists origin (
+ id bigserial not null,
+ url text not null,
+ hash text not null
+);
diff --git a/swh/core/db/tests/data/test/fail/sql/40-funcs.sql b/swh/core/db/tests/data/test/fail/sql/40-funcs.sql
new file mode 100644
index 00000000..d4dd410b
--- /dev/null
+++ b/swh/core/db/tests/data/test/fail/sql/40-funcs.sql
@@ -0,0 +1,6 @@
+create or replace function hash_sha1(text)
+ returns text
+ language sql strict immutable
+as $$
+ select encode(public.digest($1, 'sha1'), 'hex')
+$$;
diff --git a/swh/core/db/tests/test_cli.py b/swh/core/db/tests/test_cli.py
index b08ddbdb..4cb8ce43 100644
--- a/swh/core/db/tests/test_cli.py
+++ b/swh/core/db/tests/test_cli.py
@@ -3,8 +3,6 @@
# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information
-from subprocess import CalledProcessError
-
import pytest
from pytest_postgresql import factories
import yaml
@@ -117,8 +115,7 @@ def test_cli_swh_db_initialization_fail_without_creation_first(
result = cli_runner.invoke(swhdb, ["init", module_name, "--dbname", conninfo])
# Fails because we cannot connect to an inexisting db
assert result.exit_code == 1, f"Unexpected output: {result.output}"
- assert isinstance(result.exception, CalledProcessError)
- assert b'FATAL: database "inexisting-db" does not exist' in result.exception.stderr
+ assert 'FATAL: database "inexisting-db" does not exist' in result.output
def test_cli_swh_db_initialization_fail_without_extension(
@@ -136,10 +133,8 @@ def test_cli_swh_db_initialization_fail_without_extension(
# Fails as the function `public.digest` is not installed, init-admin calls is needed
# first (the next tests show such behavior)
assert result.exit_code == 1, f"Unexpected output: {result.output}"
- assert isinstance(result.exception, CalledProcessError)
assert (
- b"ERROR: function public.digest(text, unknown) does not exist"
- in result.exception.stderr
+ "ERROR: function public.digest(text, unknown) does not exist" in result.output
)
@@ -263,6 +258,30 @@ def test_cli_swh_db_create_and_init_db_new_api(
assert len(origins) == 1
+def test_cli_swh_db_init_report_sqlsh_error(
+ cli_runner,
+ postgresql,
+ mock_get_entry_points,
+ mocker,
+ tmp_path,
+):
+ """Create a db then initializing it should be ok for a "new style" datastore"""
+ module_name = "test:fail"
+
+ conninfo = craft_conninfo(postgresql)
+
+ # This initializes the schema and data
+ result = cli_runner.invoke(swhdb, ["init-admin", module_name, "--dbname", conninfo])
+ assert_result(result)
+
+ result = cli_runner.invoke(swhdb, ["init", module_name, "--dbname", conninfo])
+ assert result.exit_code == 1
+ assert (
+ "test/fail/sql/40-funcs.sql:6: "
+ "ERROR: function public.digest(text, unknown) does not exist"
+ ) in result.output
+
+
@pytest.mark.init_version(version=2)
def test_cli_swh_db_upgrade_new_api(
request,
--
GitLab