From c542a42a33fc5221020064f83f29bcb5d07312a3 Mon Sep 17 00:00:00 2001 From: Mike Gouline <1960272+gouline@users.noreply.github.com> Date: Sun, 22 Dec 2024 13:00:03 +1100 Subject: [PATCH] refactor: remove testable mangled names --- dbtmetabase/__init__.py | 2 +- dbtmetabase/_exposures.py | 64 ++++++++++++++++++++------------------- dbtmetabase/_models.py | 59 ++++++++++++++++++------------------ tests/conftest.py | 3 +- tests/test_exposures.py | 7 +++-- tests/test_models.py | 2 +- 6 files changed, 71 insertions(+), 66 deletions(-) diff --git a/dbtmetabase/__init__.py b/dbtmetabase/__init__.py index 2828e300..49c5afe4 100644 --- a/dbtmetabase/__init__.py +++ b/dbtmetabase/__init__.py @@ -9,7 +9,7 @@ ] try: - from ._version import __version__ as version # type: ignore + from ._version import __version__ as version __version__ = version except ModuleNotFoundError: diff --git a/dbtmetabase/_exposures.py b/dbtmetabase/_exposures.py index 036902ca..161a1164 100644 --- a/dbtmetabase/_exposures.py +++ b/dbtmetabase/_exposures.py @@ -88,7 +88,7 @@ def dbname(details: Mapping) -> str: return details[key] return "" - ctx = self.__Context( + ctx = _Context( model_refs={m.alias_path.lower(): m.ref for m in models if m.ref}, database_names={ d["id"]: dbname(d["details"]) for d in self.metabase.get_databases() @@ -127,7 +127,7 @@ def dbname(details: Mapping) -> str: uid=collection["id"], models=("card", "dashboard"), ): - exposure = self.__Exposure( + exposure = _Exposure( model=item["model"], uid=item["id"], label="Exposure [Unresolved Name]", @@ -153,7 +153,7 @@ def dbname(details: Mapping) -> str: f"Visualization: {entity.get('display', 'Unknown').title()}" ) - self.__exposure_card(ctx, exposure, entity) + self._exposure_card(ctx, exposure, entity) if average_query_time_ms := entity.get("average_query_time"): average_query_time_s = average_query_time_ms / 1000 @@ -179,7 +179,7 @@ def dbname(details: Mapping) -> str: continue if card := self.metabase.find_card(uid=card["id"]): - self.__exposure_card(ctx, exposure, card) + self._exposure_card(ctx, exposure, card) else: _logger.warning("Unexpected collection item '%s'", item["model"]) continue @@ -236,7 +236,7 @@ def dbname(details: Mapping) -> str: return exposures - def __exposure_card(self, ctx: __Context, exposure: __Exposure, card: Mapping): + def _exposure_card(self, ctx: _Context, exposure: _Exposure, card: Mapping): """Extracts exposures from Metabase questions.""" dataset_query = card.get("dataset_query", {}) @@ -248,7 +248,7 @@ def __exposure_card(self, ctx: __Context, exposure: __Exposure, card: Mapping): else: _logger.warning("Unsupported card type '%s'", card_type) - def __exposure_query(self, ctx: __Context, exposure: __Exposure, card: Mapping): + def __exposure_query(self, ctx: _Context, exposure: _Exposure, card: Mapping): """Extracts exposures from Metabase GUI queries.""" dataset_query = card.get("dataset_query", {}) @@ -259,7 +259,7 @@ def __exposure_query(self, ctx: __Context, exposure: __Exposure, card: Mapping): # Question based on another question source_card_uid = query_source.split("__")[-1] if source_card := self.metabase.find_card(uid=source_card_uid): - self.__exposure_card(ctx, exposure, source_card) + self._exposure_card(ctx, exposure, source_card) elif isinstance(query_source, int) and query_source in ctx.table_names: # Question based on table @@ -274,7 +274,7 @@ def __exposure_query(self, ctx: __Context, exposure: __Exposure, card: Mapping): # Question based on another question source_card_uid = join_source.split("__")[-1] if source_card := self.metabase.find_card(uid=source_card_uid): - self.__exposure_card(ctx, exposure, source_card) + self._exposure_card(ctx, exposure, source_card) continue @@ -284,7 +284,7 @@ def __exposure_query(self, ctx: __Context, exposure: __Exposure, card: Mapping): exposure.depends.add(joined_table) _logger.info("Extracted model '%s' from join", joined_table) - def __exposure_native(self, ctx: __Context, exposure: __Exposure, card: Mapping): + def __exposure_native(self, ctx: _Context, exposure: _Exposure, card: Mapping): """Extracts exposures from Metabase native queries.""" dataset_query = card.get("dataset_query", {}) @@ -410,8 +410,8 @@ def __format_exposure( return exposure - @staticmethod def __write_exposures( + self, exposures: Iterable[Mapping], output_path: str, output_grouping: Optional[str], @@ -448,24 +448,26 @@ def __write_exposures( stream=f, ) - @dc.dataclass - class __Context: - model_refs: Mapping[str, str] - database_names: Mapping[int, str] - table_names: Mapping[int, str] - - @dc.dataclass - class __Exposure: - model: str - uid: str - label: str - name: str = "" - description: str = "" - created_at: str = "" - header: str = "" - creator_name: str = "" - creator_email: str = "" - average_query_time: Optional[str] = None - last_used_at: Optional[str] = None - native_query: Optional[str] = None - depends: Set[str] = dc.field(default_factory=set) + +@dc.dataclass +class _Context: + model_refs: Mapping[str, str] + database_names: Mapping[int, str] + table_names: Mapping[int, str] + + +@dc.dataclass +class _Exposure: + model: str + uid: str + label: str + name: str = "" + description: str = "" + created_at: str = "" + header: str = "" + creator_name: str = "" + creator_email: str = "" + average_query_time: Optional[str] = None + last_used_at: Optional[str] = None + native_query: Optional[str] = None + depends: Set[str] = dc.field(default_factory=set) diff --git a/dbtmetabase/_models.py b/dbtmetabase/_models.py index 2923a4e2..ff030b64 100644 --- a/dbtmetabase/_models.py +++ b/dbtmetabase/_models.py @@ -13,12 +13,12 @@ _logger = logging.getLogger(__name__) +_SYNC_PERIOD = 5 + class ModelsMixin(metaclass=ABCMeta): """Abstraction for exporting models.""" - __SYNC_PERIOD = 5 - DEFAULT_MODELS_SYNC_TIMEOUT = 30 @property @@ -57,7 +57,7 @@ def export_models( order_fields (bool, optional): Preserve column order in dbt project. """ - ctx = self.__Context() + ctx = _Context() success = True database = self.metabase.find_database(name=metabase_database) @@ -77,9 +77,9 @@ def export_models( deadline = int(time.time()) + sync_timeout synced = False while not synced: - time.sleep(self.__SYNC_PERIOD) + time.sleep(_SYNC_PERIOD) - tables = self.__get_tables(database["id"]) + tables = self._get_metabase_tables(database["id"]) synced = True for model in models: @@ -124,7 +124,7 @@ def export_models( raise MetabaseStateError("Unable to sync models with Metabase") for model in models: - success &= self.__export_model( + success &= self._export_model( ctx=ctx, model=model, append_tags=append_tags, @@ -159,9 +159,9 @@ def export_models( if not success: raise MetabaseStateError("Non-critical errors encountered, see above") - def __export_model( + def _export_model( self, - ctx: __Context, + ctx: _Context, model: Model, append_tags: bool, docs_url: Optional[str], @@ -250,7 +250,7 @@ def __export_model( def __export_model_column_order( self, - ctx: __Context, + ctx: _Context, model: Model, api_table: Mapping, table_key: str, @@ -303,7 +303,7 @@ def __export_model_column_order( def __export_column( self, - ctx: __Context, + ctx: _Context, schema_name: str, model_name: str, column: Column, @@ -430,7 +430,7 @@ def __export_column( return success - def __get_tables(self, database_id: str) -> Mapping[str, MutableMapping]: + def _get_metabase_tables(self, database_id: str) -> Mapping[str, MutableMapping]: tables = {} metadata = self.metabase.get_database_metadata(database_id) @@ -461,8 +461,8 @@ def __get_tables(self, database_id: str) -> Mapping[str, MutableMapping]: return tables - @staticmethod def __filtered_models( + self, models: Iterable[Model], database_filter: Optional[Filter], schema_filter: Optional[Filter], @@ -479,25 +479,26 @@ def selected(m: Model) -> bool: return list(filter(selected, models)) - @dc.dataclass - class __Context: - tables: Mapping[str, MutableMapping] = dc.field(default_factory=dict) - updates: MutableMapping[str, MutableMapping] = dc.field(default_factory=dict) - def get_field(self, table_key: str, field_key: str) -> MutableMapping: - return self.tables.get(table_key, {}).get("fields", {}).get(field_key, {}) +@dc.dataclass +class _Context: + tables: Mapping[str, MutableMapping] = dc.field(default_factory=dict) + updates: MutableMapping[str, MutableMapping] = dc.field(default_factory=dict) + + def get_field(self, table_key: str, field_key: str) -> MutableMapping: + return self.tables.get(table_key, {}).get("fields", {}).get(field_key, {}) - def update(self, entity: MutableMapping, change: Mapping, label: str): - entity.update(change) + def update(self, entity: MutableMapping, change: Mapping, label: str): + entity.update(change) - key = f"{entity['kind']}.{entity['id']}" - update = self.updates.get(key, {}) - update["kind"] = entity["kind"] - update["id"] = entity["id"] - update["label"] = label + key = f"{entity['kind']}.{entity['id']}" + update = self.updates.get(key, {}) + update["kind"] = entity["kind"] + update["id"] = entity["id"] + update["label"] = label - body = update.get("body", {}) - body.update(change) - update["body"] = body + body = update.get("body", {}) + body.update(change) + update["body"] = body - self.updates[key] = update + self.updates[key] = update diff --git a/tests/conftest.py b/tests/conftest.py index 31b8113a..6802d059 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,12 +1,13 @@ import pytest +import dbtmetabase._models from tests._mocks import TMP_PATH, MockDbtMetabase, MockMetabase @pytest.fixture(name="core") def fixture_core() -> MockDbtMetabase: c = MockDbtMetabase() - c._ModelsMixin__SYNC_PERIOD = 1 # type: ignore + dbtmetabase._models._SYNC_PERIOD = 1 return c diff --git a/tests/test_exposures.py b/tests/test_exposures.py index 6a6bed65..0b00a989 100644 --- a/tests/test_exposures.py +++ b/tests/test_exposures.py @@ -4,6 +4,7 @@ import pytest import yaml +from dbtmetabase._exposures import _Context, _Exposure from tests._mocks import FIXTURES_PATH, TMP_PATH, MockDbtMetabase @@ -96,7 +97,7 @@ def test_extract_exposures_native_depends( query: str, expected: set, ): - ctx = MockDbtMetabase._ExposuresMixin__Context( # type: ignore + ctx = _Context( model_refs={ "database.schema.table0": "model0", "database.public.table1": "model1", @@ -104,12 +105,12 @@ def test_extract_exposures_native_depends( database_names={1: "database"}, table_names={}, ) - exposure = MockDbtMetabase._ExposuresMixin__Exposure( # type: ignore + exposure = _Exposure( model="card", uid="", label="", ) - core._ExposuresMixin__exposure_card( # type: ignore + core._exposure_card( ctx=ctx, exposure=exposure, card={ diff --git a/tests/test_models.py b/tests/test_models.py index 88cf954c..ce2bd597 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -80,7 +80,7 @@ def test_build_lookups(core: MockDbtMetabase): "INVENTORY.SKUS": {"SKU_ID", "PRODUCT"}, } - actual_tables = core._ModelsMixin__get_tables(database_id="2") # type: ignore + actual_tables = core._get_metabase_tables(database_id="2") assert set(actual_tables.keys()) == set(expected.keys())