From 917833f7b6faef69a3c1f2b4a5ee19a4a4ff235c Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Fri, 27 Dec 2024 10:57:39 -0500 Subject: [PATCH 1/7] [dagster-powerbi] Use Power BI translator instance in spec loader and state-backed defs --- .../power-bi/customize-power-bi-asset-defs.py | 2 +- .../dagster-powerbi/dagster_powerbi/resource.py | 15 ++++++++------- .../dagster_powerbi_tests/test_asset_specs.py | 4 +++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py b/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py index 091121656e147..f9ee870bfc334 100644 --- a/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py +++ b/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py @@ -37,7 +37,7 @@ def get_asset_spec(self, data: PowerBIContentData) -> dg.AssetSpec: power_bi_specs = load_powerbi_asset_specs( - power_bi_workspace, dagster_powerbi_translator=MyCustomPowerBITranslator + power_bi_workspace, dagster_powerbi_translator=MyCustomPowerBITranslator # type: ignore ) defs = dg.Definitions( assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace} diff --git a/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py b/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py index 16d5f7bcaeb58..473b6fb0de6ab 100644 --- a/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py +++ b/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py @@ -385,7 +385,7 @@ def build_defs( if PowerBITagSet.extract(spec.tags).asset_type == "semantic_model" else spec for spec in load_powerbi_asset_specs( - self, dagster_powerbi_translator, use_workspace_scan=False + self, dagster_powerbi_translator(), use_workspace_scan=False ) ], resources={resource_key: self}, @@ -395,13 +395,16 @@ def build_defs( @experimental def load_powerbi_asset_specs( workspace: PowerBIWorkspace, - dagster_powerbi_translator: Type[DagsterPowerBITranslator] = DagsterPowerBITranslator, + dagster_powerbi_translator: Optional[DagsterPowerBITranslator] = None, use_workspace_scan: bool = True, ) -> Sequence[AssetSpec]: """Returns a list of AssetSpecs representing the Power BI content in the workspace. Args: workspace (PowerBIWorkspace): The Power BI workspace to load assets from. + dagster_powerbi_translator (Optional[DagsterPowerBITranslator]): The translator to use + to convert Power BI content into :py:class:`dagster.AssetSpec`. + Defaults to :py:class:`DagsterPowerBITranslator`. use_workspace_scan (bool): Whether to scan the entire workspace using admin APIs at once to get all content. Defaults to True. @@ -412,7 +415,7 @@ def load_powerbi_asset_specs( return check.is_list( PowerBIWorkspaceDefsLoader( workspace=initialized_workspace, - translator_cls=dagster_powerbi_translator, + translator=dagster_powerbi_translator or DagsterPowerBITranslator(), use_workspace_scan=use_workspace_scan, ) .build_defs() @@ -424,7 +427,7 @@ def load_powerbi_asset_specs( @dataclass class PowerBIWorkspaceDefsLoader(StateBackedDefinitionsLoader[PowerBIWorkspaceData]): workspace: PowerBIWorkspace - translator_cls: Type[DagsterPowerBITranslator] + translator: DagsterPowerBITranslator use_workspace_scan: bool @property @@ -438,15 +441,13 @@ def fetch_state(self) -> PowerBIWorkspaceData: ) def defs_from_state(self, state: PowerBIWorkspaceData) -> Definitions: - translator = self.translator_cls() - all_external_data = [ *state.dashboards_by_id.values(), *state.reports_by_id.values(), *state.semantic_models_by_id.values(), ] all_external_asset_specs = [ - translator.get_asset_spec( + self.translator.get_asset_spec( PowerBITranslatorData( content_data=content, workspace_data=state, diff --git a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py index 555288ebf7498..32196a361cebd 100644 --- a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py +++ b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py @@ -99,7 +99,9 @@ def test_translator_custom_metadata(workspace_data_api_mocks: None, workspace_id workspace_id=workspace_id, ) all_asset_specs = load_powerbi_asset_specs( - workspace=resource, dagster_powerbi_translator=MyCustomTranslator, use_workspace_scan=False + workspace=resource, + dagster_powerbi_translator=MyCustomTranslator(), + use_workspace_scan=False, ) asset_spec = next(spec for spec in all_asset_specs) From 45bd1c4340d61c23ebb5855990f64c4570396832 Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Fri, 27 Dec 2024 12:17:30 -0500 Subject: [PATCH 2/7] Lint --- .../integrations/power-bi/customize-power-bi-asset-defs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py b/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py index f9ee870bfc334..a20163dd0e82b 100644 --- a/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py +++ b/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py @@ -37,7 +37,8 @@ def get_asset_spec(self, data: PowerBIContentData) -> dg.AssetSpec: power_bi_specs = load_powerbi_asset_specs( - power_bi_workspace, dagster_powerbi_translator=MyCustomPowerBITranslator # type: ignore + power_bi_workspace, + dagster_powerbi_translator=MyCustomPowerBITranslator, # type: ignore ) defs = dg.Definitions( assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace} From f80afc859a18fd7d010aaf384ac9629db7e433d9 Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Fri, 27 Dec 2024 14:36:31 -0500 Subject: [PATCH 3/7] Fix pyright --- docs/content/integrations/powerbi.mdx | 3 ++- .../project_atproto_dashboard/dashboard/definitions.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/content/integrations/powerbi.mdx b/docs/content/integrations/powerbi.mdx index 7bd09d6017b79..5f9a81b736901 100644 --- a/docs/content/integrations/powerbi.mdx +++ b/docs/content/integrations/powerbi.mdx @@ -122,7 +122,8 @@ class MyCustomPowerBITranslator(DagsterPowerBITranslator): power_bi_specs = load_powerbi_asset_specs( - power_bi_workspace, dagster_powerbi_translator=MyCustomPowerBITranslator + power_bi_workspace, + dagster_powerbi_translator=MyCustomPowerBITranslator, ) defs = dg.Definitions( assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace} diff --git a/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py b/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py index e5414bee55a1e..ce66da2d7a266 100644 --- a/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py +++ b/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py @@ -43,7 +43,7 @@ def get_semantic_model_spec(self, data: PowerBIContentData) -> dg.AssetSpec: power_bi_specs = load_powerbi_asset_specs( power_bi_workspace, - dagster_powerbi_translator=CustomDagsterPowerBITranslator, + dagster_powerbi_translator=CustomDagsterPowerBITranslator, # type: ignore ) defs = dg.Definitions(assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace}) From d84667e5f0ea950e00857d81f3b58c94bffd16a9 Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Thu, 2 Jan 2025 13:22:24 -0500 Subject: [PATCH 4/7] Add deprecation warning --- .../power-bi/customize-power-bi-asset-defs.py | 2 +- .../dagster_powerbi/resource.py | 21 +++++++++++++++---- .../dagster_powerbi_tests/test_asset_specs.py | 21 +++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py b/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py index a20163dd0e82b..d7b49b6141642 100644 --- a/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py +++ b/examples/docs_snippets/docs_snippets/integrations/power-bi/customize-power-bi-asset-defs.py @@ -38,7 +38,7 @@ def get_asset_spec(self, data: PowerBIContentData) -> dg.AssetSpec: power_bi_specs = load_powerbi_asset_specs( power_bi_workspace, - dagster_powerbi_translator=MyCustomPowerBITranslator, # type: ignore + dagster_powerbi_translator=MyCustomPowerBITranslator, ) defs = dg.Definitions( assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace} diff --git a/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py b/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py index 473b6fb0de6ab..8d17944df9b4c 100644 --- a/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py +++ b/python_modules/libraries/dagster-powerbi/dagster_powerbi/resource.py @@ -4,7 +4,7 @@ import time from dataclasses import dataclass from functools import cached_property -from typing import Any, Dict, Mapping, Optional, Sequence, Type +from typing import Any, Dict, Mapping, Optional, Sequence, Type, Union from urllib.parse import urlencode import requests @@ -21,6 +21,7 @@ from dagster._time import get_current_timestamp from dagster._utils.cached_method import cached_method from dagster._utils.security import non_secure_md5_hash_str +from dagster._utils.warnings import deprecation_warning from pydantic import Field, PrivateAttr from dagster_powerbi.translator import ( @@ -395,15 +396,17 @@ def build_defs( @experimental def load_powerbi_asset_specs( workspace: PowerBIWorkspace, - dagster_powerbi_translator: Optional[DagsterPowerBITranslator] = None, + dagster_powerbi_translator: Optional[ + Union[DagsterPowerBITranslator, Type[DagsterPowerBITranslator]] + ] = None, use_workspace_scan: bool = True, ) -> Sequence[AssetSpec]: """Returns a list of AssetSpecs representing the Power BI content in the workspace. Args: workspace (PowerBIWorkspace): The Power BI workspace to load assets from. - dagster_powerbi_translator (Optional[DagsterPowerBITranslator]): The translator to use - to convert Power BI content into :py:class:`dagster.AssetSpec`. + dagster_powerbi_translator (Optional[Union[DagsterPowerBITranslator, Type[DagsterPowerBITranslator]]]): + The translator to use to convert Power BI content into :py:class:`dagster.AssetSpec`. Defaults to :py:class:`DagsterPowerBITranslator`. use_workspace_scan (bool): Whether to scan the entire workspace using admin APIs at once to get all content. Defaults to True. @@ -411,6 +414,16 @@ def load_powerbi_asset_specs( Returns: List[AssetSpec]: The set of assets representing the Power BI content in the workspace. """ + if isinstance(dagster_powerbi_translator, type): + deprecation_warning( + subject="Support of `dagster_powerbi_translator` as a Type[DagsterPowerBITranslator]", + breaking_version="1.10", + additional_warn_text=( + "Pass an instance of DagsterPowerBITranslator or subclass to `dagster_powerbi_translator` instead." + ), + ) + dagster_powerbi_translator = dagster_powerbi_translator() + with workspace.process_config_and_initialize_cm() as initialized_workspace: return check.is_list( PowerBIWorkspaceDefsLoader( diff --git a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py index 32196a361cebd..5f5547184a1b1 100644 --- a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py +++ b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py @@ -111,6 +111,27 @@ def test_translator_custom_metadata(workspace_data_api_mocks: None, workspace_id assert "dagster/kind/powerbi" in asset_spec.tags +def test_translator_custom_metadata_legacy( + workspace_data_api_mocks: None, workspace_id: str +) -> None: + fake_token = uuid.uuid4().hex + resource = PowerBIWorkspace( + credentials=PowerBIToken(api_token=fake_token), + workspace_id=workspace_id, + ) + all_asset_specs = load_powerbi_asset_specs( + workspace=resource, + dagster_powerbi_translator=MyCustomTranslator, + use_workspace_scan=False, + ) + asset_spec = next(spec for spec in all_asset_specs) + + assert "custom" in asset_spec.metadata + assert asset_spec.metadata["custom"] == "metadata" + assert asset_spec.key.path == ["prefix", "dashboard", "Sales_Returns_Sample_v201912"] + assert "dagster/kind/powerbi" in asset_spec.tags + + @lazy_definitions def state_derived_defs_two_workspaces() -> Definitions: resource = PowerBIWorkspace( From f75ae1be06e1479b997f6c02c5fd5b2c64b7ab21 Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Thu, 2 Jan 2025 14:35:09 -0500 Subject: [PATCH 5/7] Fix pyright --- .../project_atproto_dashboard/dashboard/definitions.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py b/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py index ce66da2d7a266..06063f4a7a212 100644 --- a/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py +++ b/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py @@ -43,7 +43,6 @@ def get_semantic_model_spec(self, data: PowerBIContentData) -> dg.AssetSpec: power_bi_specs = load_powerbi_asset_specs( power_bi_workspace, - dagster_powerbi_translator=CustomDagsterPowerBITranslator, # type: ignore -) + dagster_powerbi_translator=CustomDagsterPowerBITranslator, defs = dg.Definitions(assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace}) From ffbe0c53f6f61751ca1b9ec54bab3edfbf3d35d4 Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Thu, 2 Jan 2025 14:49:42 -0500 Subject: [PATCH 6/7] Update post review --- .../dashboard/definitions.py | 1 + .../dagster_powerbi_tests/test_asset_specs.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py b/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py index 06063f4a7a212..e5414bee55a1e 100644 --- a/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py +++ b/examples/project_atproto_dashboard/project_atproto_dashboard/dashboard/definitions.py @@ -44,5 +44,6 @@ def get_semantic_model_spec(self, data: PowerBIContentData) -> dg.AssetSpec: power_bi_specs = load_powerbi_asset_specs( power_bi_workspace, dagster_powerbi_translator=CustomDagsterPowerBITranslator, +) defs = dg.Definitions(assets=[*power_bi_specs], resources={"power_bi": power_bi_workspace}) diff --git a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py index 5f5547184a1b1..eb3060d95c3a3 100644 --- a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py +++ b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py @@ -119,11 +119,15 @@ def test_translator_custom_metadata_legacy( credentials=PowerBIToken(api_token=fake_token), workspace_id=workspace_id, ) - all_asset_specs = load_powerbi_asset_specs( - workspace=resource, - dagster_powerbi_translator=MyCustomTranslator, - use_workspace_scan=False, - ) + with pytest.warns( + DeprecationWarning, + match=r"Support of `dagster_powerbi_translator` as a Type\[DagsterPowerBITranslator\]", + ): + all_asset_specs = load_powerbi_asset_specs( + workspace=resource, + dagster_powerbi_translator=MyCustomTranslator, + use_workspace_scan=False, + ) asset_spec = next(spec for spec in all_asset_specs) assert "custom" in asset_spec.metadata From 7faa2f02513306458365029cf0ef6f0728e56ec4 Mon Sep 17 00:00:00 2001 From: Maxime Armstrong Date: Thu, 2 Jan 2025 15:05:53 -0500 Subject: [PATCH 7/7] Add comment in test --- .../dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py index eb3060d95c3a3..7c250b635a44f 100644 --- a/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py +++ b/python_modules/libraries/dagster-powerbi/dagster_powerbi_tests/test_asset_specs.py @@ -123,6 +123,7 @@ def test_translator_custom_metadata_legacy( DeprecationWarning, match=r"Support of `dagster_powerbi_translator` as a Type\[DagsterPowerBITranslator\]", ): + # Pass the translator type all_asset_specs = load_powerbi_asset_specs( workspace=resource, dagster_powerbi_translator=MyCustomTranslator,