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

DSI 0.4.0 and Saved Query Exports #8950

Merged
merged 10 commits into from
Nov 1, 2023

Conversation

QMalcolm
Copy link
Contributor

@QMalcolm QMalcolm commented Oct 31, 2023

resolves #8892

Problem

dbt-semantic-interfaces recently released 0.4.0. We want to support this for 1.7.0 of dbt-core. dbt-semantic-interfaces 0.4.0 has a few changes from 0.3.0

  • where, group_by, and metrics are now nested under the query_params property on a SavedQuery
  • The SavedQuery now has a concept of exports

Solution

Begin supporting the changes.

Other things to flag

In 0.4.0 the names of metrics begin being validated by dbt-semantic-interfaces. This was always supposed to be happening, but wasn't 😬 This might break for some people. To be clear it was likely breaking in the semantic layer anyways. This just ensures that the issue is caught at parse time rather than query time.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
  • This PR includes type annotations for new and modified functions

@QMalcolm QMalcolm added the Skip Changelog Skips GHA to check for changelog file label Oct 31, 2023
@cla-bot cla-bot bot added the cla:yes label Oct 31, 2023
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (211392c) 86.50% compared to head (e30a837) 86.46%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8950      +/-   ##
==========================================
- Coverage   86.50%   86.46%   -0.04%     
==========================================
  Files         177      179       +2     
  Lines       26439    26505      +66     
==========================================
+ Hits        22870    22917      +47     
- Misses       3569     3588      +19     
Flag Coverage Δ
integration 83.28% <82.41%> (-0.10%) ⬇️
unit 64.80% <72.52%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
core/dbt/contracts/graph/unparsed.py 94.14% <100.00%> (+0.15%) ⬆️
core/dbt/parser/manifest.py 92.28% <ø> (ø)
core/dbt/parser/schema_renderer.py 95.74% <100.00%> (ø)
core/dbt/parser/schema_yaml_readers.py 92.72% <100.00%> (+0.21%) ⬆️
core/setup.py 0.00% <ø> (ø)
core/dbt/contracts/graph/semantic_layer_common.py 94.11% <94.11%> (ø)
core/dbt/contracts/graph/saved_queries.py 83.33% <83.33%> (ø)
core/dbt/contracts/graph/nodes.py 93.69% <50.00%> (-0.52%) ⬇️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@QMalcolm QMalcolm force-pushed the qmalcolm--8892-saved-query-exports-and-dsi-0.4.0 branch from 29b662e to bb5fa88 Compare October 31, 2023 20:01
@QMalcolm QMalcolm added user docs [docs.getdbt.com] Needs better documentation artifacts and removed Skip Changelog Skips GHA to check for changelog file labels Oct 31, 2023
@QMalcolm QMalcolm marked this pull request as ready for review October 31, 2023 20:37
@QMalcolm QMalcolm requested a review from a team as a code owner October 31, 2023 20:37
@QMalcolm QMalcolm requested a review from aranke October 31, 2023 20:37
@QMalcolm QMalcolm requested a review from a team as a code owner October 31, 2023 20:43
@QMalcolm QMalcolm requested review from chrismorrisette-dbt and removed request for a team October 31, 2023 20:43
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, where are we defining using DSI 0.4.x?

My PR probably needed to be updated if this goes in first, or the other way around

In dbt-labs/dbt-semantic-interfaces#186 we
renamed `group_bys` to `group_by` in DSI (which went out in DSI 0.4.0).
The original `group_bys` naming was a typo. This fixes that. This is a
non-breaking change from the core perspective because saved queries
are being introduced in 1.7.0, which has not been cut yet.
This is useful because moving it out of `nodes.py` avoids circular imports.
The protocol uses `schema_name` because on the DSI side `schema` causes
an error when in use with pydantic classes. However we want the user
to specify `schema`. This allows for that.
@QMalcolm
Copy link
Contributor Author

QMalcolm commented Nov 1, 2023

Rebasing off of main to resolve conflicts

@QMalcolm QMalcolm force-pushed the qmalcolm--8892-saved-query-exports-and-dsi-0.4.0 branch from 38226ce to e30a837 Compare November 1, 2023 00:56
@QMalcolm
Copy link
Contributor Author

QMalcolm commented Nov 1, 2023

LGTM, where are we defining using DSI 0.4.x?

My PR probably needed to be updated if this goes in first, or the other way around

We define the version in 'core/setup.py'

@QMalcolm QMalcolm merged commit ac97294 into main Nov 1, 2023
51 checks passed
@QMalcolm QMalcolm deleted the qmalcolm--8892-saved-query-exports-and-dsi-0.4.0 branch November 1, 2023 01:34
@FishtownBuildBot
Copy link
Collaborator

Opened a new issue in dbt-labs/docs.getdbt.com: dbt-labs/docs.getdbt.com#4381

Copy link
Contributor

github-actions bot commented Nov 1, 2023

The backport to 1.7.latest failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.7.latest 1.7.latest
# Navigate to the new working tree
cd .worktrees/backport-1.7.latest
# Create a new branch
git switch --create backport-8950-to-1.7.latest
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ac972948b84e3280aa5831c5a32555521714efda
# Push it to GitHub
git push --set-upstream origin backport-8950-to-1.7.latest
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.7.latest

Then, create a pull request where the base branch is 1.7.latest and the compare/head branch is backport-8950-to-1.7.latest.

github-actions bot pushed a commit that referenced this pull request Nov 1, 2023
QMalcolm added a commit that referenced this pull request Nov 1, 2023
QMalcolm added a commit that referenced this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-3255] [implementation] interface changes to dbt-core saved_query spec to add export
3 participants