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

Allow models to execute on different warehouses #488

Merged
merged 3 commits into from
Nov 10, 2023

Conversation

rcypher-databricks
Copy link
Collaborator

Resolves PECO-1182

Description

Allows models in a dbt project to run on different SQL warehouses.
The available warehouses are specified as named values in the dbt profile.
The configuration for a model can specify by name which warehouse it should run on.

Checklist

  • 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
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

@benc-db
Copy link
Collaborator

benc-db commented Oct 26, 2023

Out of curiosity, how did you validate? Do you have tests that are uncommitted, or did you look at the logs of some existing tests?

@rcypher-databricks
Copy link
Collaborator Author

I verified by modifying one of the integration tests and stepping through the code as well as adding some temporary print statements. I validated the open session parameters for a model that specifies a different warehouse than the one in the profile.

@benc-db
Copy link
Collaborator

benc-db commented Oct 26, 2023

Stylistically looks good, and to the extent I can follow, the logic looks right. Assuming we go this way, please add unit tests for the new logic, and at least 1 functional test that uses the compute definitions from the existing profiles.

@benc-db
Copy link
Collaborator

benc-db commented Oct 26, 2023

Stylistically looks good, and to the extent I can follow, the logic looks right. Assuming we go this way, please add unit tests for the new logic, and at least 1 functional test that uses the compute definitions from the existing profiles.

Per discussion with dbt, we're going to try to finish migrating to their functional tests, so no new integration tests.

@rcypher-databricks
Copy link
Collaborator Author

Definitely going to add tests if this is the implementation we decide to use. That's why its a draft PR at this point

Raise an exception if a model specifies a compute resource that is not defined in the profile.
Signed-off-by: Raymond Cypher <[email protected]>
Signed-off-by: Raymond Cypher <[email protected]>
return {"model_names": ["target3"]}


class TestSpecifyingForProjectModelsInFolder(BaseSpecifyingCompute):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know if we can specify a compute to use with models of a particular tag? This came up in a customer call where they would want to tag certain models as heavy_compute for example.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After reading the dbt docs on tags, I don't think that would work, which is probably fine. I think having the named compute approach gets us 95% of the way to what it would be if they could target compute to tags.

@rcypher-databricks rcypher-databricks merged commit 7c9fc66 into databricks:main Nov 10, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants