-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[dagster-looker] Use get_asset_spec().key in DagsterLookerApiTranslator #26048
[dagster-looker] Use get_asset_spec().key in DagsterLookerApiTranslator #26048
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
575b9d8
to
f551fa8
Compare
def get_asset_spec(self, looker_structure: LookerStructureData) -> AssetSpec: | ||
return super().get_asset_spec(looker_structure)._replace(metadata=expected_metadata) | ||
default_spec = super().get_asset_spec(looker_structure) | ||
return default_spec.replace_attributes( |
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.
nit: this would probably be easier done as merge_attributes, which will automatically merge the metadata dictionary
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.
sweet
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.
Updated in 47484bf
f551fa8
to
47484bf
Compare
Summary & Motivation
Like #26027 but for Looker API
How I Tested These Changes
Updated unit test with BK