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
ExternalAssetNode #4742
ExternalAssetNode #4742
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/elementl/dagster/9c26BsqcqaS1ryhf8aCvbZQ8CZY7 |
|
||
|
||
@whitelist_for_serdes | ||
class ExternalAssetDefinition( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love the name ExternalAssetDefinition
in the absence of the asset
decorator. Also, since this is internal-facing only, maybe something like ExternalAssetNode
?
Just thinking things through: we can no longer assume there exists at most one asset node per repository / asset_key, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking things through: we can no longer assume there exists at most one asset node per repository / asset_key, right?
I think that's right
42966de
to
c48eec0
Compare
c48eec0
to
02adf9e
Compare
02adf9e
to
f171199
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple renaming suggestions
return self.external_repository_data.external_asset_graph_data | ||
|
||
def get_external_asset_definition(self, asset_key: AssetKey): | ||
matching = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_external_asset_dependencies / get_external_asset_node?
) | ||
|
||
|
||
def external_asset_defs_from_defs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
external_asset_graph_from_pipeline_defs?
f171199
to
9af9310
Compare
No description provided.