Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[external assets] source assets -> external assets #18217

Closed

Conversation

smackesey
Copy link
Collaborator

@smackesey smackesey commented Nov 22, 2023

Summary & Motivation

Internally convert source assets to external assets. This happens at the level of RepositoryData-- we do the conversion here instead of in Definitions so that conversion works for both @repository and Definitions.

"Source assets" no longer exist as a concept at the level of ExternalAssetNode, but still exist over the GQL API to keep the representation in Dagster UI constant. A new isExternal field is also added to GQL asset nodes.

A caveat is that auto_observe_interval_minutes can not be perfectly translated to an auto-materialize policy, because there is no way to capture "every N minutes" exactly as a cron expression (and AutoMaterializeRule.materialize_on_cron is the closest option available). I used a mapping that gets us pretty close though where we round to the nearest minute/hour/day for the number of minutes.

How I Tested These Changes

@smackesey
Copy link
Collaborator Author

smackesey commented Nov 22, 2023

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

Join @smackesey and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Nov 22, 2023

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-9n1yxcik2-elementl.vercel.app
https://sean-convert-source-assets.core-storybook.dagster-docs.io

Built with commit 5ebeac7.
This pull request is being automatically deployed with vercel-action

@smackesey smackesey force-pushed the sean/convert-source-assets branch from 94579d5 to 76e746d Compare November 23, 2023 13:36
@smackesey smackesey force-pushed the sean/external-assets-observe-result branch from 5503f7f to 33d754c Compare January 29, 2024 22:39
@smackesey smackesey force-pushed the sean/convert-source-assets branch from 76e746d to 1aa3b00 Compare January 29, 2024 22:39
@smackesey smackesey force-pushed the sean/external-assets-observe-result branch from 33d754c to fc5f310 Compare February 1, 2024 17:21
@smackesey smackesey force-pushed the sean/convert-source-assets branch from 1aa3b00 to 2efc378 Compare February 1, 2024 17:21
This was referenced Feb 1, 2024
@smackesey smackesey force-pushed the sean/external-assets-observe-result branch from fc5f310 to 375c613 Compare February 1, 2024 17:45
@smackesey smackesey force-pushed the sean/convert-source-assets branch from 2efc378 to 1bfc23d Compare February 1, 2024 17:45
@smackesey smackesey force-pushed the sean/external-assets-observe-result branch 4 times, most recently from ce7dd71 to dc76212 Compare February 2, 2024 17:59
@smackesey smackesey force-pushed the sean/convert-source-assets branch 2 times, most recently from e3bb98c to 8b0fb54 Compare February 2, 2024 19:30
@smackesey smackesey force-pushed the sean/external-assets-observe-result branch 2 times, most recently from 1625c87 to 1670c1f Compare February 5, 2024 14:33
Base automatically changed from sean/external-assets-observe-result to master February 5, 2024 15:21
@smackesey smackesey force-pushed the sean/convert-source-assets branch 2 times, most recently from e3772e2 to 0fb2e11 Compare February 7, 2024 11:58
Copy link

github-actions bot commented Feb 7, 2024

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-ao3tft7y7-elementl.vercel.app
https://sean-convert-source-assets.dagster.dagster-docs.io

Direct link to changed pages:

@smackesey smackesey force-pushed the sean/convert-source-assets branch 2 times, most recently from a8a9ee5 to db14c71 Compare February 7, 2024 17:46
@smackesey smackesey changed the base branch from master to sean/allow-mixed-asset-jobs February 8, 2024 13:31
@smackesey smackesey force-pushed the sean/convert-source-assets branch from db14c71 to f5c0284 Compare February 8, 2024 13:31
@smackesey smackesey force-pushed the sean/convert-source-assets branch from f5c0284 to 5ebeac7 Compare February 8, 2024 13:57
@smackesey smackesey changed the base branch from sean/allow-mixed-asset-jobs to sean/remove-source-asset-from-jobs February 9, 2024 18:55
@smackesey smackesey force-pushed the sean/convert-source-assets branch from 5ebeac7 to 24d608d Compare February 9, 2024 18:55
@smackesey smackesey force-pushed the sean/remove-source-asset-from-jobs branch 6 times, most recently from 3233518 to 08255d1 Compare February 12, 2024 16:15
@smackesey smackesey force-pushed the sean/remove-source-asset-from-jobs branch 2 times, most recently from 5b214ac to 3bfbef8 Compare February 12, 2024 20:48
@smackesey smackesey force-pushed the sean/remove-source-asset-from-jobs branch from 3bfbef8 to 05b414b Compare March 20, 2024 15:58
@smackesey
Copy link
Collaborator Author

Superseded by another stack.

@smackesey smackesey closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant