-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Move manifest nodes to dbt/artifacts #9538
Merged
Merged
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
33f475d
Move ParsedNodeMandatory; need to fix up a few things from core
gshank 6d5aaeb
cleanup, remove NodeAndTestConfig and NodeConfig from model_config.py
gshank 73b4f9b
Remove some unnecessar type: ignores
gshank 755e41c
Move MacroDependsOn and Docs (circular reference)
gshank 6d48ccc
Merge branch 'main' into manifest_nodes_to_artifacts
gshank 6313fa2
ParsedNode, initial attempt
gshank 9c28930
Move CompiledNode
gshank a8f3690
AnalysisNode, HookNode, ModelNode, ModelConfig
gshank 81479b1
SqlNode and SeedNode
gshank e4d554f
Move SingularTestNode and TestConfig
gshank 6029dcf
Move GenericTestNode
gshank 433c556
Move SnapshotNode and SnapshotConfig
gshank 4d6c698
Changie
gshank 82a110a
Merge branch 'main' into gs/manifest_nodes_to_artifacts
gshank 4974d13
Merge branch 'main' into gs/manifest_nodes_to_artifacts
gshank 520f3dd
Merge branch 'main' into gs/manifest_nodes_to_artifacts
gshank d5e7402
Remove duplicate Hook class, use ModelConfig in config dictionary
gshank 2fabc84
Merge branch 'main' into gs/manifest_nodes_to_artifacts
gshank 9cedcd3
Remove Node from the names of a bunch of resources
gshank 5c4c528
Rename some intermediate node classes
gshank d43409a
Fix merge error
gshank 3a4ead8
Change NodeType.Model back to having NodeConfig
gshank 82dbeac
Separate out manifest nodes into individual files
gshank b0646c5
Use ModelConfig in model_config type table
gshank 07229e2
validate patch.config['access']
gshank File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
kind: Under the Hood | ||
body: Move manifest nodes to artifacts | ||
time: 2024-02-07T12:23:42.909049-05:00 | ||
custom: | ||
Author: gshank | ||
Issue: "9388" |
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
We could rename these to ParsedResource (& ParsedResourceMandatory) + CompiledResource and define them in https://github.com/dbt-labs/dbt-core/blob/main/core/dbt/artifacts/resources/base.py to be more consistent with the base + graph resource refactors.
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.
We can't add anything to classes in base.py because they aren't versioned. Are we sure that this is what we want?
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.
Discussed offline: there isn't really a clear policy shaking out to discern whether these type of base resource classes should be defined in v1/ or one directory above in base.py. Let's revisit this in later on once we have backward/forward compatibility considered and tested (issue: #9615) , potentially as part of a more holistic 'final' reorganization push.