Only create schemas for selected nodes (#1239) #1258
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
January 23, 2019 16:35
- pass selected uids along to the before_run method for schema creation - only schemas used by selected nodes are created - fix some big issues with check_schema_exists (such as: it does not work) - integration test changes to test this
beckjake
force-pushed
the
fix/dont-create-unused-schemas
branch
from
9e6ca80
to
9ff8705
Compare
beckjake
force-pushed
the
fix/dont-create-unused-schemas
branch
from
a4b77e4
to
7d332aa
Compare
|
Ignore that last force-push, it's not a change! Just reverting an accidental wrong-branch commit. |
drewbanin
reviewed
Jan 24, 2019
| @@ -241,4 +241,4 @@ def check_schema_exists(self, database, schema, model_name=None): | |||
| kwargs={'database': database, 'schema': schema}, | |||
| connection_name=model_name | |||
| ) | |||
| return results[0] > 0 | |||
| return results[0][0] > 0 | |||
There was a problem hiding this comment.
the existing check_schema_exists was wrong and did not work.
| schemas = set() | ||
| for node in manifest.nodes.values(): | ||
| if node.unique_id not in selected_uids: | ||
| continue | ||
| if cls.is_refable(node) and not cls.is_ephemeral(node): |
There was a problem hiding this comment.
is this going to cause a problem with Sources? I imagine sources are refable (idk if you actually made that change in the corresponding PR), but we definitely wouldn't want to touch source schemas at all! Might be worth being more explicit here? I'd only care about schemas for:
- models
- seeds
- archives (to come in Wilt Chamberlain)
No change required necessarily, but wanted to surface it because i saw it :)
There was a problem hiding this comment.
Sources are not refable, so no.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1239
This PR passes along the set of selected UIDs to the
before_runmethod of the chosen Runner class, which is then used to filter out the nodes that dbt needs to build schemas for. We still unconditionally create the default schema, for snowflake.Also I have no idea what was up with
check_schema_exists, but it's fixed.For tests, I just modified some of the existing graph selection tests, which seemed sufficient.