[1/n] Add `with_source_code_references` fn to add source location metadata to assets #21465

benpankow · 2024-04-26T21:28:39Z

Summary

Rework of #12096. Adds a with_source_code_references wrapper which attaches source code location metadata to the assets which it wraps.

Stacked PRs handle displaying it in the UI.

Right now, the code source metadata is structured as a list of file references, each includes a path to the source file and and line number, along with an optional label to be shown in the UI. Users can choose to manually add these to their asset definitions, either via decorator or as part of an asset factory.

@asset(
    metadata={
        **CodeReferencesMetadataSet(
            source_data=CodeReferencesMetadataValue(
                code_references=[
                    LocalFileSource(
                        file_path=..., line_number=...
                    ),
                    LocalFileSource(
                        label="SQL file", file_path=..., line_number=...
                    )
                ]
            )
        )
    }
)
def my_multiple_source_asset() -> pd.DataFrame: ...

The with_source_code_references wrapper can be used to automatically attach local file paths and line numbers to assets using inspect. This behavior may later become default-on, but for now is explicitly opt-in.

@asset 
def my_asset() -> pd.DataFrame:
  ...

@asset 
def my_other_asset() -> pd.DataFrame:
  ...

defs = Definitions(
  assets=with_source_code_references([my_asset, my_other_asset])
)

Test Plan

New unit tests.

benpankow · 2024-04-26T21:28:55Z

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

Join @benpankow and the rest of your teammates on Graphite

benpankow · 2024-04-29T22:44:35Z

js_modules/dagster-ui/packages/ui-core/src/metadata/MetadataEntry.tsx

@@ -257,6 +257,8 @@ export const MetadataEntry = ({
          </IconButton>
        </Group>
      );
+    case 'SourceMetadataEntry':


minimal change to placate bk, actual tsx changes are stacked

github-actions · 2024-04-29T22:47:39Z

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-doucdpro0-elementl.vercel.app
https://benpankow-add-code-source-metadata-wrapper.core-storybook.dagster-docs.io

Built with commit 8d6aefa.
This pull request is being automatically deployed with vercel-action

python_modules/dagster/dagster/_core/definitions/metadata/__init__.py

python_modules/dagster/dagster_tests/asset_defs_tests/test_asset_defs_source_metadata.py

queue

python_modules/dagster/dagster/_core/definitions/metadata/source_code.py

## Summary Some restructuring/cleanup to make #21465 a bit easier

queue

benpankow · 2024-05-06T17:51:44Z

PR is updated to move the label to the LocalFileCodeReference class & just use a list instead. Updated description to match.

have it default to the last component of the file path (e.g. if the file is /at/some/folder/foo.py the label would default to foo.py), which I think will be a suitable default in 99% of cases

This is a good idea, will incorporate in downstream UI PRs.

While I think with_source_code_references is a good approach to experiment with and opt-in into this feature, I believe our goal should be to have this applied by default.

Agreed - right now the feature is explicitly opt-in because of the potential cost of inspect calls (& because the output filepaths/line numbers may be misleading for more advanced cases like asset factories), but once we stabilize the impl I think it makes sense to roll out by default.

queue

schrockn

Lovely. Thanks for working through this.

github-actions · 2024-05-07T17:20:34Z

Deploy preview for dagster-university ready!

✅ Preview
https://dagster-university-912my3521-elementl.vercel.app
https://benpankow-add-code-source-metadata-wrapper.dagster-university.dagster-docs.io

Built with commit 33d4e57.
This pull request is being automatically deployed with vercel-action

benpankow · 2024-05-08T15:36:19Z

Merge activity

May 8, 8:36 AM PDT: @benpankow started a stack merge that includes this pull request via Graphite.
May 8, 8:38 AM PDT: Graphite rebased this pull request as part of a merge.
May 8, 8:39 AM PDT: @benpankow merged this pull request with Graphite.

@asset

## Summary & Motivation Displays a button link to view an asset in the user's editor, for code link metadata specified in #21465. Also hides this metadata from the table. ```python @asset def my_asset() -> pd.DataFrame: ... with_code_source([my_asset]) ``` <img width="955" alt="Screenshot 2024-04-26 at 3 41 53 PM" src="https://github.com/dagster-io/dagster/assets/10215173/8d846b4d-377b-4a4e-a9bd-69c30eb9bee4"> If the user specifies multiple files, they are displayed with a dropdown: ```python @asset( metadata={ **SourceDataMetadataSet( source_paths={ DEFAULT_SOURCE_FILE_KEY: SourcePathMetadataSet( path_to_module=..., path_from_module=..., line_number=... ), "SQL model": SourcePathMetadataSet( path_to_module=..., path_from_module=..., line_number=... ), } ) } ) def my_two_source_asset() -> pd.DataFrame: ... ``` <img width="248" alt="Screenshot 2024-04-26 at 4 23 55 PM" src="https://github.com/dagster-io/dagster/assets/10215173/16ec33ef-8ade-4b52-97ad-6b0a491b426e">

## Summary Some restructuring/cleanup to make dagster-io#21465 a bit easier

@asset

…adata to assets (dagster-io#21465) ## Summary Rework of dagster-io#12096. Adds a `with_source_code_references` wrapper which attaches source code location metadata to the assets which it wraps. Stacked PRs handle displaying it in the UI. Right now, the code source metadata is structured as a list of file references, each includes a path to the source file and and line number, along with an optional label to be shown in the UI. Users can choose to manually add these to their asset definitions, either via decorator or as part of an asset factory. ```python @asset( metadata={ **CodeReferencesMetadataSet( source_data=CodeReferencesMetadataValue( code_references=[ LocalFileSource( file_path=..., line_number=... ), LocalFileSource( label="SQL file", file_path=..., line_number=... ) ] ) ) } ) def my_multiple_source_asset() -> pd.DataFrame: ... ``` The `with_source_code_references` wrapper can be used to automatically attach local file paths and line numbers to assets using `inspect`. This behavior may later become default-on, but for now is explicitly opt-in. ```python @asset def my_asset() -> pd.DataFrame: ... @asset def my_other_asset() -> pd.DataFrame: ... defs = Definitions( assets=with_source_code_references([my_asset, my_other_asset]) ) ``` ## Test Plan New unit tests.

@asset

## Summary & Motivation Displays a button link to view an asset in the user's editor, for code link metadata specified in dagster-io#21465. Also hides this metadata from the table. ```python @asset def my_asset() -> pd.DataFrame: ... with_code_source([my_asset]) ``` <img width="955" alt="Screenshot 2024-04-26 at 3 41 53 PM" src="https://github.com/dagster-io/dagster/assets/10215173/8d846b4d-377b-4a4e-a9bd-69c30eb9bee4"> If the user specifies multiple files, they are displayed with a dropdown: ```python @asset( metadata={ **SourceDataMetadataSet( source_paths={ DEFAULT_SOURCE_FILE_KEY: SourcePathMetadataSet( path_to_module=..., path_from_module=..., line_number=... ), "SQL model": SourcePathMetadataSet( path_to_module=..., path_from_module=..., line_number=... ), } ) } ) def my_two_source_asset() -> pd.DataFrame: ... ``` <img width="248" alt="Screenshot 2024-04-26 at 4 23 55 PM" src="https://github.com/dagster-io/dagster/assets/10215173/16ec33ef-8ade-4b52-97ad-6b0a491b426e">

benpankow marked this pull request as draft April 26, 2024 21:30

This was referenced Apr 26, 2024

[2/n] add frontend code link protocol customization in user settings #21467

Merged

[3/n] display link to editor in the asset catalog #21468

Merged

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from ebafe76 to e032f1d Compare April 26, 2024 23:27

benpankow marked this pull request as ready for review April 26, 2024 23:28

benpankow requested a review from sryza April 29, 2024 15:47

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from e032f1d to d0ff45e Compare April 29, 2024 15:50

benpankow requested a review from rexledesma April 29, 2024 16:36

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from 04406c7 to 859f329 Compare April 29, 2024 22:20

benpankow commented Apr 29, 2024

View reviewed changes

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from 0b354bc to d46bc1d Compare April 30, 2024 17:29

sryza previously requested changes Apr 30, 2024

View reviewed changes

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from d46bc1d to bda13a9 Compare May 2, 2024 00:08

benpankow mentioned this pull request May 2, 2024

separate out __init__ file for metadata pkg #21576

Merged

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from bda13a9 to 9fb26a5 Compare May 2, 2024 00:23

benpankow changed the base branch from master to benpankow/restructure-metadata-py May 2, 2024 00:23

benpankow requested a review from sryza May 2, 2024 00:24

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from 0a18e77 to f7932d1 Compare May 2, 2024 00:54

sryza previously requested changes May 2, 2024

View reviewed changes

python_modules/dagster/dagster/_core/definitions/metadata/source_code.py Outdated Show resolved Hide resolved

python_modules/dagster/dagster/_core/definitions/metadata/source_code.py Outdated Show resolved Hide resolved

Base automatically changed from benpankow/restructure-metadata-py to master May 2, 2024 16:00

benpankow added a commit that referenced this pull request May 2, 2024

separate out __init__ file for metadata pkg (#21576)

ef0341b

## Summary Some restructuring/cleanup to make #21465 a bit easier

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from f7932d1 to 4afa85d Compare May 2, 2024 16:01

benpankow requested a review from sryza May 2, 2024 18:02

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from 0a9304d to c9a6c28 Compare May 2, 2024 18:03

schrockn requested review from schrockn and removed request for schrockn May 2, 2024 19:28

benpankow requested review from schrockn and sryza May 6, 2024 17:51

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch 2 times, most recently from 8e20595 to fceb4be Compare May 6, 2024 23:22

benpankow mentioned this pull request May 6, 2024

[4/n] add python api for replacing local file references with source control links #21675

Merged

schrockn approved these changes May 7, 2024

View reviewed changes

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from fceb4be to 33d4e57 Compare May 7, 2024 17:15

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from 33d4e57 to 180d3e7 Compare May 7, 2024 23:23

benpankow mentioned this pull request May 8, 2024

[5/n] frontend for url code references #21713

Merged

benpankow added 8 commits May 8, 2024 15:37

restructure

b1e8861

fix import

b72658a

rename

93e9499

Rename + remove js changes for now

1b7a43f

update gql class names

0b814b0

update code references dict to list

5ce5e6a

fix test

9277d53

minimal ts changes

8d6aefa

benpankow force-pushed the benpankow/add-code-source-metadata-wrapper branch from 180d3e7 to 8d6aefa Compare May 8, 2024 15:37

benpankow merged commit f8ba248 into master May 8, 2024
1 of 2 checks passed

benpankow deleted the benpankow/add-code-source-metadata-wrapper branch May 8, 2024 15:39

benpankow mentioned this pull request May 15, 2024

export code reference classes at dagster package #21881

Open

danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024

separate out __init__ file for metadata pkg (dagster-io#21576)

63a472f

## Summary Some restructuring/cleanup to make dagster-io#21465 a bit easier

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1/n] Add `with_source_code_references` fn to add source location metadata to assets #21465

[1/n] Add `with_source_code_references` fn to add source location metadata to assets #21465

benpankow commented Apr 26, 2024 •

edited

Loading

benpankow commented Apr 26, 2024 •

edited

Loading

benpankow Apr 29, 2024

github-actions bot commented Apr 29, 2024 •

edited

Loading

benpankow commented May 6, 2024

schrockn left a comment

github-actions bot commented May 7, 2024

benpankow commented May 8, 2024 •

edited

Loading

[1/n] Add with_source_code_references fn to add source location metadata to assets #21465

[1/n] Add with_source_code_references fn to add source location metadata to assets #21465

Conversation

benpankow commented Apr 26, 2024 • edited Loading

Summary

Test Plan

benpankow commented Apr 26, 2024 • edited Loading

benpankow Apr 29, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 29, 2024 • edited Loading

benpankow commented May 6, 2024

schrockn left a comment

Choose a reason for hiding this comment

github-actions bot commented May 7, 2024

benpankow commented May 8, 2024 • edited Loading

Merge activity

[1/n] Add `with_source_code_references` fn to add source location metadata to assets #21465

[1/n] Add `with_source_code_references` fn to add source location metadata to assets #21465

benpankow commented Apr 26, 2024 •

edited

Loading

benpankow commented Apr 26, 2024 •

edited

Loading

github-actions bot commented Apr 29, 2024 •

edited

Loading

benpankow commented May 8, 2024 •

edited

Loading