-
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-dbt] Spec-ify dbt cloud integration #26662
[dagster-dbt] Spec-ify dbt cloud integration #26662
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
045075a
to
d07cd21
Compare
4599035
to
cd1dfdc
Compare
d07cd21
to
d74987c
Compare
5f32d81
to
c10af5c
Compare
@@ -890,137 +881,6 @@ def _validate_asset_keys( | |||
) | |||
|
|||
|
|||
def get_asset_deps( |
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.
so wonderful
deps=dbt_dependencies, | ||
# TODO: In the future, allow the IO manager to be specified. | ||
io_manager_key=None, | ||
# generate specs for each executed node |
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.
filler comment
manifest=None, | ||
select=" ".join( | ||
f"fqn:{'.'.join(dbt_nodes[unique_id]['fqn'])}" for unique_id in executed_unique_ids | ||
), | ||
) | ||
|
||
return AssetsDefinitionCacheableData( | ||
# TODO: In the future, we should allow additional upstream assets to be specified. |
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.
while we're here can we get rid of this / make a linear ticket (but I think this is possible via map asset specs now, so not really necessary
keys_by_output_name={ | ||
output_name: asset_key for asset_key, (output_name, _) in asset_outs.items() | ||
}, | ||
keys_by_output_name={spec.key.to_python_identifier(): spec.key for spec in specs}, |
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.
somewhat surprised we don't also have to include checks here, but it doesn't look like we did before either
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 few comments but generally awesome
d74987c
to
35c9420
Compare
c10af5c
to
be8c8bb
Compare
be8c8bb
to
ca1bbf9
Compare
Summary & Motivation
Updates the dbt cloud integration to be more spec-native, taking advantage of existing utilities for generating specs from a dbt project.
This results in the ability to delete a large amount of code.
How I Tested These Changes
Changelog
NOCHANGELOG