From 34877aa746ec7fe902bb1d9cdd1e5bbec1a5fe99 Mon Sep 17 00:00:00 2001
From: Felix Uellendall <feluelle@users.noreply.github.com>
Date: Tue, 10 Jan 2023 14:51:14 +0100
Subject: [PATCH] Add error message for validate connections (#1554)

# Description

## What is the current behavior?

Currently, we are not giving any hints to the user about an
invalid/failed connection.

closes: #1535

## What is the new behavior?

Print an error message when validating a connection.

## Does this introduce a breaking change?

Yes, kind of. We are not erroring out anymore when the file path
internally could not get resolved in case of sqlite connections. This is
so that an invalid sqlite connection does not block you from running any
generate/validate/run commands. Previously it was raising an exception
which led to an immediate exit.

### Checklist
- [x] Created tests which fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary

Co-authored-by: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
---
 sql-cli/sql_cli/__main__.py                    |  6 +-----
 sql-cli/sql_cli/connections.py                 |  9 ++++++---
 sql-cli/tests/config/invalid/configuration.yml |  6 +++---
 sql-cli/tests/test___main__.py                 | 13 +++++++++----
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/sql-cli/sql_cli/__main__.py b/sql-cli/sql_cli/__main__.py
index c7439b17a..3e9ea871e 100644
--- a/sql-cli/sql_cli/__main__.py
+++ b/sql-cli/sql_cli/__main__.py
@@ -155,11 +155,7 @@ def validate(
     set_verbose_mode(verbose)
     project_dir_absolute = resolve_project_dir(project_dir)
     project = Project(project_dir_absolute)
-    try:
-        project.load_config(environment=env)
-    except FileNotFoundError as file_not_found_error:
-        log.exception(file_not_found_error.args[0])
-        rprint(f"[bold red]{file_not_found_error}[/bold red]")
+    project.load_config(environment=env)
 
     rprint(f"Validating connection(s) for environment '{env}'")
     validate_connections(connections=project.connections, connection_id=connection)
diff --git a/sql-cli/sql_cli/connections.py b/sql-cli/sql_cli/connections.py
index 612946596..3ca36634a 100644
--- a/sql-cli/sql_cli/connections.py
+++ b/sql-cli/sql_cli/connections.py
@@ -32,9 +32,10 @@ def convert_to_connection(conn: dict[str, Any], data_dir: Path) -> Connection:
         # Try resolving with data directory
         resolved_host = data_dir / connection["host"]
         if not resolved_host.is_file():
-            raise FileNotFoundError(
-                f"The relative file path {connection['host']} was resolved into {resolved_host}"
-                " but it's a failed resolution as the path does not exist."
+            log.error(
+                "The relative file path %s was resolved into %s but it does not exist.",
+                connection["host"],
+                resolved_host,
             )
         connection["host"] = resolved_host.as_posix()
 
@@ -56,6 +57,8 @@ def validate_connections(connections: list[Connection], connection_id: str | Non
             success_status, message = _is_valid(connection)
             status = "[bold green]PASSED[/bold green]" if success_status else "[bold red]FAILED[/bold red]"
             rprint(f"Validating connection {connection.conn_id:{CONNECTION_ID_OUTPUT_STRING_WIDTH}}", status)
+            if not success_status:
+                rprint(f"  [bold red]Error: {message}[/bold red]")
             formatted_message = (
                 f"[bold green]{message}[/bold green]" if success_status else f"[bold red]{message}[/bold red]"
             )
diff --git a/sql-cli/tests/config/invalid/configuration.yml b/sql-cli/tests/config/invalid/configuration.yml
index ce7b3dfc0..aa16ab4fd 100644
--- a/sql-cli/tests/config/invalid/configuration.yml
+++ b/sql-cli/tests/config/invalid/configuration.yml
@@ -2,6 +2,6 @@ connections:
   - conn_id: sqlite_non_existent_host_path
     conn_type: sqlite
     host: imdb2.db
-    login:
-    password:
-    schema:
+  - conn_id: unknown_hook_type
+    conn_type: sqlite2
+    host: imdb.db
diff --git a/sql-cli/tests/test___main__.py b/sql-cli/tests/test___main__.py
index 44fb284ce..08586d5f8 100644
--- a/sql-cli/tests/test___main__.py
+++ b/sql-cli/tests/test___main__.py
@@ -345,12 +345,17 @@ def test_validate_all(env, initialised_project_with_test_config):
 
 
 @pytest.mark.parametrize(
-    "env,connection",
+    "env,connection,message",
     [
-        ("invalid", "sqlite_non_existent_host_path"),
+        ("invalid", "sqlite_non_existent_host_path", "Error: Sqlite db does not exist!"),
+        ("invalid", "unknown_hook_type", 'Error: Unknown hook type "sqlite2"'),
+    ],
+    ids=[
+        "sqlite_non_existent_host_path",
+        "unknown_hook_type",
     ],
 )
-def test_validate_invalid(env, connection, initialised_project_with_invalid_config, logger):
+def test_validate_invalid(env, connection, message, initialised_project_with_invalid_config, logger):
     result = runner.invoke(
         app,
         [
@@ -364,7 +369,7 @@ def test_validate_invalid(env, connection, initialised_project_with_invalid_conf
     )
     assert result.exit_code == 0, result.exception
     assert f"Validating connection(s) for environment '{env}'" in result.stdout
-    assert "Error: Config file does not contain given connection" in result.stdout
+    assert message in result.stdout
 
 
 @pytest.mark.parametrize(