[Chore] minor fixes to improve messages (#857)

* Fix latest not removed bug Signed-off-by: Ching Yi, Chan <[email protected]> * Improve dx for run.json input Signed-off-by: Ching Yi, Chan <[email protected]> * Add dbt-version to diagnose Signed-off-by: Ching Yi, Chan <[email protected]> * Improve dbt schema compatiable messages Signed-off-by: Ching Yi, Chan <[email protected]> --------- Signed-off-by: Ching Yi, Chan <[email protected]>
InfuseAI · Aug 24, 2023 · 3d44e4b · 3d44e4b
1 parent 93e5a11
commit 3d44e4b
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 18 deletions.
diff --git a/piperider_cli/cli.py b/piperider_cli/cli.py
@@ -1,8 +1,10 @@
+import json
 import os.path
 import sys
 
 import click
 import sentry_sdk
+from click import Context, Parameter
 from rich.console import Console
 
 import piperider_cli.dbtutil as dbtutil
@@ -23,6 +25,7 @@
 from piperider_cli.recipes import RecipeConfiguration, configure_recipe_execution_flags, is_recipe_dry_run
 from piperider_cli.runner import Runner
 from piperider_cli.validator import Validator
+import typing as t
 
 release_version = __version__ if sentry_env != 'development' else None
 
@@ -77,6 +80,53 @@ def _add_options(func):
     return _add_options
 
 
+class RunDataPath(click.Path):
+
+    def __init__(self):
+        super().__init__(
+            exists=True,
+            file_okay=True,
+            dir_okay=False,
+            writable=False,
+            readable=True,
+            resolve_path=True,
+            allow_dash=False,
+            path_type=None,
+            executable=False
+        )
+
+    def convert(
+            self, value: t.Any, param: t.Optional["Parameter"], ctx: t.Optional["Context"]
+    ) -> t.Any:
+        rv = value
+
+        try:
+            data = self.read_file(rv)
+            self.validate(data)
+        except BaseException as e:
+            self.fail(f'{e} from: "{value}"')
+            pass
+
+        return super().convert(rv, param, ctx)
+
+    def read_file(self, rv):
+        try:
+            with open(rv) as fh:
+                return json.loads(fh.read())
+        except BaseException:
+            raise ValueError('failed to read data')
+
+    def validate(self, data):
+        if not isinstance(data, t.Dict):
+            raise ValueError('type mismatched, we expected a dict')
+        required_keys = {'tables', 'datasource'}
+        if not set(data.keys()).issuperset(required_keys):
+            raise ValueError(f'missing one of required keys: {required_keys}')
+
+        if 'dbt' in data and data.get('dbt', {}).get('manifest') is None:
+            raise ValueError('there is no dbt manifest data in the run.json')
+
+
 @click.group(name="piperider", invoke_without_command=True)
 @click.pass_context
 def cli(ctx: click.Context):
@@ -313,7 +363,7 @@ def generate_assertions(**kwargs):
 
 
 @cli.command(short_help='Generate a report.', cls=TrackCommand)
-@click.option('--input', default=None, type=click.Path(exists=True), help='Specify the raw result file.')
+@click.option('--input', default=None, type=RunDataPath(), help='Specify the raw result file.')
 @click.option('--output', '-o', default=None, type=click.STRING, help='Directory to save the results.')
 @click.option('--report-dir', default=None, type=click.STRING, help='Use a different report directory.')
 @add_options(debug_option)
@@ -323,8 +373,8 @@ def generate_report(**kwargs):
 
 
 @cli.command(short_help='Compare two existing reports.', cls=TrackCommand)
-@click.option('--base', default=None, type=click.Path(exists=True), help='Specify the base report file.')
-@click.option('--target', default=None, type=click.Path(exists=True), help='Specify the report file to be compared.')
+@click.option('--base', default=None, type=RunDataPath(), help='Specify the base report file.')
+@click.option('--target', default=None, type=RunDataPath(), help='Specify the report file to be compared.')
 @click.option('--last', default=None, is_flag=True, help='Compare the last two reports.')
 @click.option('--datasource', default=None, type=click.STRING, metavar='DATASOURCE_NAME',
               help='Specify the datasource.')
@@ -428,7 +478,7 @@ def cloud(**kwargs):
 
 
 @cloud.command(short_help='Upload a report to the PipeRider Cloud.', cls=TrackCommand)
-@click.option('--run', type=click.Path(exists=True), help='Specify the raw result file.')
+@click.option('--run', type=RunDataPath(), help='Specify the raw result file.')
 @click.option('--report-dir', default=None, type=click.STRING, help='Use a different report directory.')
 @click.option('--datasource', default=None, type=click.STRING, metavar='DATASOURCE_NAME',
               help='Specify the datasource.')

diff --git a/piperider_cli/compare_report.py b/piperider_cli/compare_report.py
@@ -193,8 +193,13 @@ def _update_implicit_and_explicit_changeset(self):
             self.summary_change_set = SummaryChangeSet(self._base, self._target)
         except BaseException as e:
             console = Console()
-            console.print(
-                f'[bold yellow]Warning:[/bold yellow] {e}. Got problem to generate changeset.')
+            console.print('[bold yellow]Warning:[/bold yellow]')
+            if isinstance(e, ValueError) and e.args:
+                for line in e.args[0]:
+                    console.print(f'  {line}', end='\n')
+            else:
+                console.print(e)
+            console.print('\nGot problem to generate changeset.')
 
     def _update_github_pr_info(self):
         if os.environ.get('GITHUB_EVENT_PATH'):

diff --git a/piperider_cli/dbt/__init__.py b/piperider_cli/dbt/__init__.py
@@ -55,7 +55,7 @@ def __eq__(self, other):
         return self.dbt_version.release[:2] == self.as_version(other).release[:2]
 
     def __str__(self):
-        return self.dbt_version
+        return ".".join([str(x) for x in list(self.dbt_version.release)])
 
 
 dbt_version = DbtVersionTool()
diff --git a/piperider_cli/dbt/list_task.py b/piperider_cli/dbt/list_task.py
@@ -85,9 +85,6 @@ def _load_manifest_version_14(data: Dict):
 
 
 def _load_manifest_version_15(manifest: Dict):
-    from dbt.exceptions import IncompatibleSchemaError
-
-    # return WritableManifest.read_and_check_versions(manifest_path)
     data = manifest
 
     # Check metadata version. There is a class variable 'dbt_schema_version', but
@@ -98,10 +95,14 @@ def _load_manifest_version_15(manifest: Dict):
             previous_schema_version = data["metadata"]["dbt_schema_version"]
             # cls.dbt_schema_version is a SchemaVersion object
             if not WritableManifest.is_compatible_version(previous_schema_version):
-                raise IncompatibleSchemaError(
-                    expected=str(WritableManifest.dbt_schema_version),
-                    found=previous_schema_version,
-                )
+                messages = [
+                    f'Current dbt (version: {dbt_version}) could not recognize the schema in the manifest file. ',
+                    f'  supported: "{WritableManifest.dbt_schema_version}"',
+                    f'  unknown:   "{previous_schema_version}"',
+                    'Please re-generate the manifest with the compatible version.',
+                ]
+                raise ValueError(messages)
+
     return WritableManifest.upgrade_schema_version(data)
 
 

diff --git a/piperider_cli/utils.py b/piperider_cli/utils.py
@@ -9,7 +9,7 @@ def remove_link(link_path):
             if os.path.exists(link_path):
                 subprocess.run(["rmdir", link_path], shell=True, check=True)
         else:
-            if os.path.exists(link_path):
+            if os.path.exists(link_path) or os.path.islink(link_path):
                 os.unlink(link_path)
     except OSError as e:
         raise e

diff --git a/piperider_cli/validator.py b/piperider_cli/validator.py
@@ -97,8 +97,13 @@ def check_function(self, configurator: Configuration) -> (bool, str):
             type = ds.type_name
 
             if dbt:  # dbt provider
-                self.console.print(f'  DBT: {ds.type_name} > {dbt["profile"]} > {dbt["target"]} ', end='')
+                from piperider_cli.dbt import dbt_version
+                self.console.print('  DBT: ', end='')
                 self.console.print('[[bold green]OK[/bold green]]')
+                self.console.print(f'    Version: {dbt_version}')
+                self.console.print(f'    Adapter: {ds.type_name}')
+                self.console.print(f'    Profile: {dbt["profile"]}')
+                self.console.print(f'    Target:  {dbt["target"]} ')
 
             self.console.print(f'  Name: {name}')
             self.console.print(f'  Type: {type}')
@@ -107,11 +112,11 @@ def check_function(self, configurator: Configuration) -> (bool, str):
             if err:
                 all_passed = False
                 reason = err
-                self.console.print(f'  connector: [[bold red]FAILED[/bold red]] reason: {err}')
+                self.console.print(f'  Connector: [[bold red]FAILED[/bold red]] reason: {err}')
                 self.console.print(f'\n{escape(err.hint)}\n')
                 continue
             else:
-                self.console.print('  connector: [[bold green]OK[/bold green]]')
+                self.console.print('  Connector: [[bold green]OK[/bold green]]')
 
             try:
                 ds.verify_connection()