Merge pull request #9 from Eppo-exp/v0.0.3

added tests for non Eppo dbt configuration files, safer loading, desc…

Author: lukasgoetzweiss

eppo_metrics_sync/dbt_model_parser.py

@@ -1,7 +1,6 @@
 from collections import Counter
 from itertools import chain
 
-
 class DbtModelParser():
 
     def __init__(self, model, dbt_model_prefix):
@@ -44,14 +43,14 @@ def _parse_one_column(self, column):
                 self.eppo_facts.append({
                     "name": column["name"],
                     "column": column["name"],
-                    "description": column["description"]
+                    "description": column.get("description", "")
                 })
 
             if 'eppo_property' in tags:
                 self.eppo_properties.append({
                     "name": column["name"],
                     "column": column["name"],
-                    "description": column["description"]
+                    "description": column.get("description", "")
                 })
 
 
@@ -131,7 +130,12 @@ def format(self):
         }
 
     def build(self):
-        self.parse_columns()
-        self.validate()
-        self.format()
-        return self.eppo_fact_source
+        if isinstance(self.model, dict):
+            model_tags = self.model.get('tags', [])
+            if 'eppo_fact_source' in model_tags:
+                self.parse_columns()
+                self.validate()
+                self.format()
+                return self.eppo_fact_source
+        else:
+            raise ValueError(f"Expected model to be a dictionary, got model = {self.model}")

eppo_metrics_sync/eppo_metrics_sync.py

@@ -11,6 +11,8 @@
 )
 
 from eppo_metrics_sync.dbt_model_parser import DbtModelParser
+from eppo_metrics_sync.helper import load_yaml
+
 
 API_ENDPOINT = 'https://eppo.cloud/api/v1/metrics/sync'
 
@@ -35,38 +37,36 @@ def __init__(
         with open(schema_path) as schema_file:
             self.schema = json.load(schema_file)
 
+
     def load_eppo_yaml(self, path):
-        with open(path, 'r') as yaml_file:
-            yaml_data = yaml.safe_load(yaml_file)
-            if 'fact_sources' in yaml_data:
-                self.fact_sources.extend(yaml_data['fact_sources'])
-            if 'metrics' in yaml_data:
-                self.metrics.extend(yaml_data['metrics'])
+        yaml_data = load_yaml(path)
+        if 'fact_sources' in yaml_data:
+            self.fact_sources.extend(yaml_data['fact_sources'])
+        if 'metrics' in yaml_data:
+            self.metrics.extend(yaml_data['metrics'])
 
     def load_dbt_yaml(self, path):
         if not self.dbt_model_prefix:
             raise ValueError('Must specify dbt_model_prefix when schema_type=dbt-model')
-        with open(path, 'r') as yaml_file:
-            yaml_data = yaml.safe_load(yaml_file)
+        yaml_data = load_yaml(path)
         models = yaml_data.get('models')
         if models:
             for model in models:
-                self.fact_sources.append(
-                    DbtModelParser(model, self.dbt_model_prefix).build()
-                )
+                dbt_model_parser = DbtModelParser(model, self.dbt_model_prefix).build()
+                if dbt_model_parser:
+                    self.fact_sources.append(dbt_model_parser)
 
     def yaml_is_valid(self, yaml_path):
         """
         Validate a single YAML file against the schema
 
         """
-        with open(yaml_path, 'r') as yaml_file:
-            data = yaml.safe_load(yaml_file)
-            try:
-                jsonschema.validate(data, self.schema)
-                return {"passed": True}
-            except jsonschema.exceptions.ValidationError as e:
-                return {"passed": False, "error_message": e}
+        data = load_yaml(yaml_path)
+        try:
+            jsonschema.validate(data, self.schema)
+            return {"passed": True}
+        except jsonschema.exceptions.ValidationError as e:
+            return {"passed": False, "error_message": e}
 
     def read_yaml_files(self):
         # Recursively scan the directory for YAML files and load valid ones

eppo_metrics_sync/helper.py

@@ -0,0 +1,13 @@
+import yaml
+
+def load_yaml(path):
+    try:
+        with open(path, 'r') as file:
+            content = yaml.safe_load(file)
+            return content
+    except yaml.YAMLError as e:
+        raise ValueError(f"Error loading YAML file '{path}': {e}")
+    except Exception as e:
+        raise ValueError(f"Unexpected error loading file '{path}': {e}")
+
+

setup.py

@@ -2,7 +2,7 @@
 
 setup(
     name='eppo_metrics_sync',
-    version='0.0.2',
+    version='0.0.3',
     packages=find_packages(),
     install_requires=[
         'PyYAML', 'jsonschema', 'requests'

tests/test_cli.py

@@ -13,10 +13,6 @@ def runner(args):
         return result
     return runner
 
-"""def test_cli_with_valid_directory(run_cli):
-    result = run_cli(['tests/yaml/valid'])
-    assert result.returncode == 0
-"""
 def test_cli_dryrun_option(run_cli):
     result = run_cli(['tests/yaml/valid', '--dryrun'])
     assert result.returncode == 0

tests/test_validation_dbt.py

@@ -3,7 +3,7 @@
 from eppo_metrics_sync.validation import unique_names, valid_fact_references, aggregation_is_valid
 from eppo_metrics_sync.eppo_metrics_sync import EppoMetricsSync
 
-test_yaml_dir = "tests/yaml/dbt/invalid"
+test_yaml_dir = "tests/yaml/dbt/"
 
 
 def test_invalid_entity_tag():
@@ -17,7 +17,7 @@ def test_invalid_entity_tag():
         AssertionError, 
         match = "Invalid entity tag eppo_entity:anonymous_user:foo in model revenue"
         ):
-        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "/invalid_entity_tag.yml")
+        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "invalid/invalid_entity_tag.yml")
 
 
 def test_missing_entity():
@@ -31,7 +31,7 @@ def test_missing_entity():
         ValueError, 
         match = 'At least 1 column must have tag "eppo_entity:<entity_name>"'
         ):
-        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "/missing_entity.yml")
+        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "invalid/missing_entity.yml")
 
 
 def test_missing_timestamp():
@@ -45,7 +45,7 @@ def test_missing_timestamp():
         ValueError, 
         match = 'Exactly 1 column must be have tag "eppo_timestamp"'
         ):
-        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "/missing_timestamp.yml")
+        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "invalid/missing_timestamp.yml")
 
 
 def test_overlapping_tags():
@@ -59,14 +59,39 @@ def test_overlapping_tags():
         ValueError, 
         match = 'The following columns had tags to multiple Eppo fields: gross_revenue'
         ):
-        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "/overlapping_tags.yml")
+        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "invalid/overlapping_tags.yml")
 
-# test that package handles yml without 'models' member gracefully
+
+def test_model_is_not_a_dictionary():
+
+    eppo_metrics_sync = EppoMetricsSync(
+        directory = None, 
+        schema_type = 'dbt-model',
+        dbt_model_prefix = 'foo'
+    )
+    with pytest.raises(
+        ValueError, 
+        match = 'Expected model to be a dictionary, got model = r'
+        ):
+        eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "invalid/model_is_not_a_dictionary.yml")
+
+# test that the package handles yml without 'models' member gracefully
+def test_no_model_tag():
+
+    eppo_metrics_sync = EppoMetricsSync(
+        directory = None, 
+        schema_type = 'dbt-model',
+        dbt_model_prefix = 'foo'
+    )
+    eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "valid/no_model_property.yml")
+
+
+# test that the package handles dbt models without eppo tags gracefully
 def test_no_model_tag():
 
     eppo_metrics_sync = EppoMetricsSync(
         directory = None, 
         schema_type = 'dbt-model',
         dbt_model_prefix = 'foo'
     )
-    eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "/no_model_tag.yml")
+    eppo_metrics_sync.load_dbt_yaml(path = test_yaml_dir + "valid/no_dbt_tags.yml")

tests/yaml/dbt/invalid/model_is_not_a_dictionary.yml

@@ -0,0 +1,3 @@
+version: 2
+
+models: revenue

tests/yaml/dbt/valid/no_model_tag.yml renamed to tests/yaml/dbt/valid/no_dbt_tags.yml

@@ -1,28 +1,19 @@
 
 version: 2
 
-foo:
+models:
   - name: revenue
     description: "An example revenue model"
-    tags:
-      - eppo_fact_source
     columns:
       - name: revenue_id
         description: "The primary key for this table"
         tests:
           - unique
           - not_null
       - name: anonymous_id
-        tags:
-          - eppo_entity:anonymous_user
       - name: purchase_timestamp
         description: "The time at which the revenue was created"
         tests:
           - not_null
-        tags:
-          - eppo_timestamp
       - name: gross_revenue
         description: "The gross revenue amount"
-        tags: 
-          - eppo_fact
-          - eppo_property

tests/yaml/dbt/invalid/no_model_tag.yml renamed to tests/yaml/dbt/valid/no_model_property.yml

@@ -4,25 +4,16 @@ version: 2
 foo:
   - name: revenue
     description: "An example revenue model"
-    tags:
-      - eppo_fact_source
     columns:
       - name: revenue_id
         description: "The primary key for this table"
         tests:
           - unique
           - not_null
       - name: anonymous_id
-        tags:
-          - eppo_entity:anonymous_user
       - name: purchase_timestamp
         description: "The time at which the revenue was created"
         tests:
           - not_null
-        tags:
-          - eppo_timestamp
       - name: gross_revenue
         description: "The gross revenue amount"
-        tags: 
-          - eppo_fact
-          - eppo_property