-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] fix dbt row count behavior on warehouses, test row count code on snowflake & bigquery #21952
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @benpankow and the rest of your teammates on Graphite |
query_result_table = query_result[1] | ||
# some adapters do not output the column names, so we need | ||
# to index by position | ||
row_count = query_result_table[0][0] |
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.
Was tested manually before it was changed to index-by-row-name, which evidently works in duckdb but not Snowflake unfortunately
91f1cac
to
a7f490e
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.
For the OSS nightly pipeline, can we ensure that it notifies a highly visible Slack channel (e.g. #topic-dbt, #project-data-catalog), on failure?
I want to ensure that these failures aren't owned by the person just monitoring the nightly pipeline.
python_modules/libraries/dagster-dbt/dagster_dbt_tests/core/test_row_count_postprocessing.py
Outdated
Show resolved
Hide resolved
a7f490e
to
441d6e1
Compare
Set up to notify to #eng-buildkite-nightly so that it's targeted in a somewhat generic way and not drowned out (e.g. other teams can start using it + setting up notifs). |
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.
Worth updating the title to make it clear this is fixing something and not just adding tests
…t code on snowflake & bigquery (#21952) ## Summary Adds Snowflake & BQ tests of new row-count functionality and fixes bug with `Table` return on certain warehouses (incl Snowflake). Added to the new OSS nightly pipeline introduced in #21956 to avoid racking up unnecessary spend. We do already run some tests on these warehouses as part of normal builds for their respective integrations, so maybe it's fine to do the same here - open to either. ## Test Plan Nightly build run: https://buildkite.com/dagster/full-moon-with-face-dagster-nightly/builds/9
…t code on snowflake & bigquery (#21952) ## Summary Adds Snowflake & BQ tests of new row-count functionality and fixes bug with `Table` return on certain warehouses (incl Snowflake). Added to the new OSS nightly pipeline introduced in #21956 to avoid racking up unnecessary spend. We do already run some tests on these warehouses as part of normal builds for their respective integrations, so maybe it's fine to do the same here - open to either. ## Test Plan Nightly build run: https://buildkite.com/dagster/full-moon-with-face-dagster-nightly/builds/9
Summary
Adds Snowflake & BQ tests of new row-count functionality and fixes bug with
Table
return on certain warehouses (incl Snowflake).Added to the new OSS nightly pipeline introduced in #21956 to avoid racking up unnecessary spend. We do already run some tests on these warehouses as part of normal builds for their respective integrations, so maybe it's fine to do the same here - open to either.
Test Plan
Nightly build run:
https://buildkite.com/dagster/full-moon-with-face-dagster-nightly/builds/9