[dagster-tableau] Update custom translator example in Tableau docs #26029

maximearmstrong · 2024-11-20T02:29:46Z

Summary & Motivation

Update the custom translator example in the Tableau docs to reflect get_asset_spec().key.

This PR stack will be merged after #25941 lands, so that replace_attributes() can be called as AssetSpec().replace_attributes()

maximearmstrong · 2024-11-20T02:29:57Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2024-11-20T02:33:36Z

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-2sgq2rhwi-elementl.vercel.app
https://maxime-update-tableau-docs-asset-key-translator.dagster.dagster-docs.io

Direct link to changed pages:

https://dagster-docs-2sgq2rhwi-elementl.vercel.app
https://maxime-update-tableau-docs-asset-key-translator.dagster.dagster-docs.io/integrations/tableau

benpankow · 2024-11-20T06:16:05Z

examples/docs_snippets/docs_snippets/integrations/tableau/customize-tableau-asset-defs.py

+        # We create the default asset spec using super()
+        default_spec = super().get_asset_spec(data)
+        # We customize the metadata and asset key prefix for all assets, including sheets
+        return replace_attributes(


Do we want to use default_spec.replace_attributes, once that merges?

I think we can reduce the verbosity of this a bit once that merges - no need to set default_spec & can just do return super().get_asset_spec(data).replace_attributes(....)

Do we want to use default_spec.replace_attributes, once that merges?

Definitely - it's mentioned in the PR description, but I could have added a comment here to make it clearer. I will wait for #25941 to land before merging the PR stack.

I think we can reduce the verbosity of this a bit once that merges - no need to set default_spec & can just do return super().get_asset_spec(data).replace_attributes(....)

I think it works if we keep the comment and note after the code snippet.

I think we can reduce the verbosity of this a bit once that merges - no need to set default_spec & can just do return super().get_asset_spec(data).replace_attributes(....)

Actually, we reuse default_spec in the parameters of replace_attributes, so we should keep super().get_asset_spec(data) and replace_attributes(....) to avoid calling super() again to access the asset key. Otherwise we get this:

return super().get_asset_spec(data).replace_attributes( key=super().get_asset_spec(data).key.with_prefix("my_prefix") )

Also, having default_spec = super().get_asset_spec(data) puts the emphasis on creating the default spec first.

benpankow · 2024-11-20T06:17:27Z

examples/docs_snippets/docs_snippets/integrations/tableau/customize-tableau-asset-defs.py

+        # We customize the team owner tag only for sheets
+        return replace_attributes(default_spec, owners=["team:my_team"])
+
+    def get_asset_spec(self, data: TableauContentData) -> dg.AssetSpec:


Does it make sense for a user to want to override both get_asset_spec and get_sheet_spec? I find this a little confusing since I would imagine the typical case would be to either override one or more of the "object-level" spec methods, or just get_asset_spec?

Maybe we ought to just consolidate on overriding get_asset_spec?

That's fair - will update

Done in 5eeb24c

neverett

One tiny copyedit, otherwise looks good to me!

neverett · 2024-11-20T16:08:54Z

docs/content/integrations/tableau.mdx

@@ -104,17 +104,18 @@ defs = dg.Definitions(assets=[*tableau_specs], resources={"tableau": tableau_wor

 ### Customize asset definition metadata for Tableau assets

-By default, Dagster will generate asset keys for each Tableau asset based on its type and name and populate default metadata. You can further customize asset properties by passing a custom <PyObject module="dagster_tableau" object="DagsterTableauTranslator" /> subclass to the <PyObject module="dagster_tableau" method="load_tableau_asset_specs" /> function. This subclass can implement methods to customize the asset keys or specs for each Tableau asset type.
+By default, Dagster will generate asset specs for each Tableau asset based on its type and populate default metadata. You can further customize asset properties by passing a custom <PyObject module="dagster_tableau" object="DagsterTableauTranslator" /> subclass to the <PyObject module="dagster_tableau" method="load_tableau_asset_specs" /> function. This subclass can implement methods to customize the asset specs for each Tableau asset type.


Suggested change

By default, Dagster will generate asset specs for each Tableau asset based on its type and populate default metadata. You can further customize asset properties by passing a custom <PyObject module="dagster_tableau" object="DagsterTableauTranslator" /> subclass to the <PyObject module="dagster_tableau" method="load_tableau_asset_specs" /> function. This subclass can implement methods to customize the asset specs for each Tableau asset type.

By default, Dagster will generate asset specs for each Tableau asset based on its type, and populate default metadata. You can further customize asset properties by passing a custom <PyObject module="dagster_tableau" object="DagsterTableauTranslator" /> subclass to the <PyObject module="dagster_tableau" method="load_tableau_asset_specs" /> function. This subclass can implement methods to customize the asset specs for each Tableau asset type.

Done in b80eee2

docs/content/integrations/tableau.mdx

## Summary & Motivation Like #26029 but for Sigma

…26050) ## Summary & Motivation Like #26029 but for Looker API

This was referenced Nov 20, 2024

[dagster-tableau] Use get_asset_spec().key in DagsterTableauTranslator #26027

Merged

[dagster-tableau] Mark get_asset_key as deprecated in DagsterTableauTranslator #26028

Merged

maximearmstrong force-pushed the maxime/deprecate-get-asset-key-tableau-translator branch from 1afa7bc to 142f9ce Compare November 20, 2024 02:44

maximearmstrong force-pushed the maxime/update-tableau-docs-asset-key-translator branch from fed411e to 5dfc4b2 Compare November 20, 2024 02:44

maximearmstrong self-assigned this Nov 20, 2024

maximearmstrong marked this pull request as ready for review November 20, 2024 02:48

maximearmstrong requested a review from neverett as a code owner November 20, 2024 02:48

maximearmstrong requested review from benpankow, dpeng817 and OwenKephart November 20, 2024 02:48

benpankow approved these changes Nov 20, 2024

View reviewed changes

benpankow reviewed Nov 20, 2024

View reviewed changes

maximearmstrong force-pushed the maxime/deprecate-get-asset-key-tableau-translator branch from 142f9ce to 43ea996 Compare November 20, 2024 14:08

maximearmstrong force-pushed the maxime/update-tableau-docs-asset-key-translator branch from 5dfc4b2 to 5eeb24c Compare November 20, 2024 14:08

neverett added the docs-to-migrate Docs to migrate to new docs site label Nov 20, 2024

neverett approved these changes Nov 20, 2024

View reviewed changes

maximearmstrong force-pushed the maxime/deprecate-get-asset-key-tableau-translator branch from 43ea996 to 2ac721a Compare November 20, 2024 17:58

maximearmstrong force-pushed the maxime/update-tableau-docs-asset-key-translator branch from 7268d91 to b80eee2 Compare November 20, 2024 17:58

maximearmstrong force-pushed the maxime/deprecate-get-asset-key-tableau-translator branch from 2ac721a to de029db Compare November 25, 2024 19:08

maximearmstrong force-pushed the maxime/update-tableau-docs-asset-key-translator branch from b80eee2 to c34154e Compare November 25, 2024 19:08

This was referenced Nov 25, 2024

[dagster-sigma] Update custom translator example in Sigma docs #26037

Merged

[dagster-looker] Update custom API translator example in Looker docs #26050

Merged

[dagster-powerbi] Update custom translator example in Power BI docs #26053

Open

maximearmstrong force-pushed the maxime/deprecate-get-asset-key-tableau-translator branch from de029db to 3c9210b Compare November 25, 2024 19:33

maximearmstrong force-pushed the maxime/update-tableau-docs-asset-key-translator branch from c34154e to 3abd4e3 Compare November 25, 2024 19:34

Base automatically changed from maxime/deprecate-get-asset-key-tableau-translator to master November 25, 2024 19:57

[dagster-tableau] Update custom translator example in Tableau docs

d6e7b3a

maximearmstrong added 4 commits November 25, 2024 14:57

Move sheet customization

3e05ed4

Fix typo

3a98937

Update copy

d883f23

Use AssetSpec.replace_attributes in docs

4d3e03e

maximearmstrong force-pushed the maxime/update-tableau-docs-asset-key-translator branch from 3abd4e3 to 4d3e03e Compare November 25, 2024 19:57

graphite-app bot reviewed Nov 25, 2024

View reviewed changes

docs/content/integrations/tableau.mdx Show resolved Hide resolved

maximearmstrong merged commit 94bcd2a into master Nov 25, 2024
2 checks passed

maximearmstrong deleted the maxime/update-tableau-docs-asset-key-translator branch November 25, 2024 20:20

maximearmstrong added a commit that referenced this pull request Nov 25, 2024

[dagster-sigma] Update custom translator example in Sigma docs (#26037)

8048220

## Summary & Motivation Like #26029 but for Sigma

maximearmstrong added a commit that referenced this pull request Nov 25, 2024

[dagster-looker] Update custom API translator example in Looker docs (#…

5339b5e

…26050) ## Summary & Motivation Like #26029 but for Looker API

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dagster-tableau] Update custom translator example in Tableau docs #26029

[dagster-tableau] Update custom translator example in Tableau docs #26029

maximearmstrong commented Nov 20, 2024 •

edited

Loading

maximearmstrong commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

benpankow Nov 20, 2024

benpankow Nov 20, 2024

maximearmstrong Nov 20, 2024 •

edited

Loading

maximearmstrong Nov 20, 2024 •

edited

Loading

benpankow Nov 20, 2024

maximearmstrong Nov 20, 2024

maximearmstrong Nov 20, 2024

neverett left a comment

neverett Nov 20, 2024

maximearmstrong Nov 20, 2024

[dagster-tableau] Update custom translator example in Tableau docs #26029

[dagster-tableau] Update custom translator example in Tableau docs #26029

Conversation

maximearmstrong commented Nov 20, 2024 • edited Loading

Summary & Motivation

maximearmstrong commented Nov 20, 2024 • edited Loading

github-actions bot commented Nov 20, 2024 • edited Loading

benpankow Nov 20, 2024

Choose a reason for hiding this comment

benpankow Nov 20, 2024

Choose a reason for hiding this comment

maximearmstrong Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

maximearmstrong Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

benpankow Nov 20, 2024

Choose a reason for hiding this comment

maximearmstrong Nov 20, 2024

Choose a reason for hiding this comment

maximearmstrong Nov 20, 2024

Choose a reason for hiding this comment

neverett left a comment

Choose a reason for hiding this comment

neverett Nov 20, 2024

Choose a reason for hiding this comment

maximearmstrong Nov 20, 2024

Choose a reason for hiding this comment

maximearmstrong commented Nov 20, 2024 •

edited

Loading

maximearmstrong commented Nov 20, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

maximearmstrong Nov 20, 2024 •

edited

Loading

maximearmstrong Nov 20, 2024 •

edited

Loading