Fixes discovered in sandbox testing (#203)

Makefile

@@ -59,6 +59,20 @@ dev-install: build
 		&& python3 -m pip install dist/dbt_metabase-*-py3-none-any.whl
 .PHONY: dev-install
 
-dev-sandbox:
-	( cd sandbox && docker-compose up --build ; docker-compose down )
-.PHONY: dev-sandbox
+dev-sandbox-up:
+	( cd sandbox && docker-compose up --build --detach --wait )
+.PHONY: dev-sandbox-up
+
+dev-sandbox-models:
+	( source sandbox/.env && python3 -m dbtmetabase models \
+		--dbt-manifest-path sandbox/target/manifest.json \
+		--dbt-database $$POSTGRES_DB \
+		--metabase-url http://localhost:$$MB_PORT \
+		--metabase-username $$MB_USER \
+		--metabase-password $$MB_PASSWORD \
+		--metabase-database $$POSTGRES_DB )
+.PHONY: dev-sandbox-models
+
+dev-sandbox-down:
+	( cd sandbox && docker-compose down )
+.PHONY: dev-sandbox-up

dbtmetabase/__main__.py

@@ -238,7 +238,7 @@ def wrapper(
         metabase_session_id: Optional[str],
         metabase_verify: bool,
         metabase_cert: Optional[str],
-        metabase_http_timeout: int,
+        metabase_timeout: int,
         verbose: bool,
         **kwargs,
     ):
@@ -260,7 +260,7 @@ def wrapper(
                 session_id=metabase_session_id,
                 verify=metabase_verify,
                 cert=metabase_cert,
-                http_timeout=metabase_http_timeout,
+                http_timeout=metabase_timeout,
             ),
             **kwargs,
         )
@@ -294,15 +294,6 @@ def wrapper(
     type=click.STRING,
     help="Target database name in Metabase.",
 )
-@click.option(
-    "--metabase-sync/--metabase-sync-skip",
-    "metabase_sync",
-    envvar="METABASE_SYNC",
-    show_envvar=True,
-    default=True,
-    show_default=True,
-    help="Attempt to synchronize Metabase schema with local models.",
-)
 @click.option(
     "--metabase-sync-timeout",
     metavar="SECS",

dbtmetabase/metabase.py

@@ -125,9 +125,10 @@ def execute(self):
                 json=update["body"],
             )
             logger.info(
-                "Update to %s %s applied successfully",
+                "API %s/%s updated successfully: %s",
                 update["kind"],
                 update["id"],
+                ", ".join(update.get("body", {}).keys()),
             )
 
         if not success:
@@ -180,8 +181,10 @@ def _export_model(self, model: MetabaseModel) -> bool:
         body_table = {}
 
         # Update if specified, otherwise reset one that had been set
-        if api_table.get("display_name") != model_display_name and (
-            model_display_name or api_table.get("display_name") != api_table.get("name")
+        api_display_name = api_table.get("display_name")
+        if api_display_name != model_display_name and (
+            model_display_name
+            or not self._friendly_equal(api_display_name, api_table.get("name"))
         ):
             body_table["display_name"] = model_display_name
 
@@ -258,7 +261,7 @@ def _export_column(
 
             if not target_table or not target_field:
                 logger.info(
-                    "Passing on fk resolution for %s. Target field %s was not resolved during dbt model parsing.",
+                    "Skipping FK resolution for %s table, %s field not resolved during dbt parsing",
                     table_key,
                     target_field,
                 )
@@ -279,7 +282,7 @@ def _export_column(
                     fk_target_field_id = fk_target_field.get("id")
                     if fk_target_field.get(semantic_type_key) != "type/PK":
                         logger.info(
-                            "Target field %s will be set to PK for %s column FK",
+                            "API field/%s will become PK (for %s column FK)",
                             fk_target_field_id,
                             column_name,
                         )
@@ -291,13 +294,13 @@ def _export_column(
                         )
                     else:
                         logger.info(
-                            "Target field %s is already PK, needed for %s column",
+                            "API field/%s is already PK (for %s column FK)",
                             fk_target_field_id,
                             column_name,
                         )
                 else:
                     logger.error(
-                        "Unable to find foreign key target %s.%s",
+                        "Unable to find PK for %s.%s column FK",
                         target_table,
                         target_field,
                     )
@@ -315,9 +318,10 @@ def _export_column(
         body_field: MutableMapping[str, Optional[Any]] = {}
 
         # Update if specified, otherwise reset one that had been set
-        if api_field.get("display_name") != column_display_name and (
+        api_display_name = api_field.get("display_name")
+        if api_display_name != column_display_name and (
             column_display_name
-            or api_field.get("display_name") != api_field.get("name")
+            or not self._friendly_equal(api_display_name, api_field.get("name"))
         ):
             body_field["display_name"] = column_display_name
 
@@ -346,8 +350,9 @@ def _export_column(
             body_field["settings"] = settings
 
         # Allow explicit null type to override detected one
-        if api_field.get(semantic_type_key) != column.semantic_type and (
-            column.semantic_type or column.semantic_type is NullValue
+        api_semantic_type = api_field.get(semantic_type_key)
+        if (column.semantic_type and api_semantic_type != column.semantic_type) or (
+            column.semantic_type is NullValue and api_semantic_type
         ):
             body_field[semantic_type_key] = column.semantic_type or None
 
@@ -365,7 +370,7 @@ def _load_tables(self, database_id: str) -> Mapping[str, MutableMapping]:
         metadata = self.client.api(
             "get",
             f"/api/database/{database_id}/metadata",
-            params={"include_hidden": True},
+            params={"include_hidden": "true"},
         )
 
         bigquery_schema = metadata.get("details", {}).get("dataset-id")
@@ -394,6 +399,24 @@ def _load_tables(self, database_id: str) -> Mapping[str, MutableMapping]:
 
         return tables
 
+    def _friendly_equal(self, a: Optional[str], b: Optional[str]) -> bool:
+        """Equality test for parameters susceptible to Metabase "friendly names".
+
+        For example, "Some Name" is a friendly name for "some_name".
+
+        Args:
+            a (Optional[str]): Possibly-friendly string.
+            b (Optional[str]): Possibly-friendly string.
+
+        Returns:
+            bool: True if strings are equal normalization.
+        """
+
+        def normalize(x: Optional[str]) -> str:
+            return (x or "").replace(" ", "_").replace("-", "_").lower()
+
+        return normalize(a) == normalize(b)
+
 
 class _ExtractExposuresJob(_MetabaseClientJob):
     _RESOURCE_VERSION = 2
@@ -788,7 +811,7 @@ def __init__(
 
         self.session.mount(
             self.url,
-            http_adapter or HTTPAdapter(max_retries=Retry(total=3, backoff_factor=0.5)),
+            http_adapter or HTTPAdapter(max_retries=Retry(total=3, backoff_factor=1)),
         )
 
         if not session_id:

sandbox/requirements.txt

@@ -1,3 +1,3 @@
-molot
-requests
-dbt-postgres
+molot>=1.0.0,<1.1.0
+requests>=2.26.0,<3.0.0
+dbt-postgres>=1.7.4,<2.0.0

setup.py

@@ -30,9 +30,7 @@ def requires_from_file(filename: str) -> list:
     packages=find_packages(exclude=["tests"]),
     test_suite="tests",
     install_requires=requires_from_file("requirements.txt"),
-    extras_require={
-        "test": requires_from_file("requirements-test.txt"),
-    },
+    tests_require=requires_from_file("requirements-test.txt"),
     classifiers=[
         "Intended Audience :: Developers",
         "License :: OSI Approved :: MIT License",

tests/fixtures/sample_project/models/schema.yml

@@ -31,7 +31,7 @@ models:
           metabase.display_name: order_count
           metabase.semantic_type: null
 
-      - name: total_order_amount
+      - name: customer_lifetime_value
         description: Total value (AUD) of a customer's orders
 
   - name: orders