Schema yaml rendering and source overrides #2357

beckjake · 2020-04-24T17:08:47Z

resolves #2269
resolves #2283
resolves #2287

Description

This PR accomplishes a few related tasks:

All of schema.yml is now rendered at parse time (except tests and descriptions)
- split the config rendering logic up for the different files it handles
The docs context now has access to everything schema.yml does
Dependency sources can be overridden in schema.yml files by adding the key overrides
- To accommodate this, some structural changes:
  - re-ordered source parsing. both sources and patches are stored during parsing, and source tests are not rendered until after parsing is complete.
  - This means source tests (and less importantly, sources themselves) can't participate in partial parsing
  - Split the manifest into nodes and sources
  - Sources have more meaningful FQNs now
- Snapshot freshness overrides do the same merge that source tables do to sources, everything else clobbers
Schema.yml, profiles.yml, and dbt_project.yml values that are rendered are all now rendered in the native env.

Checklist

I have signed the CLA
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 next" section.

beckjake · 2020-04-24T17:28:38Z

core/dbt/contracts/graph/manifest.py


    def build_flat_graph(self):
        """This attribute is used in context.common by each node, so we want to
        only build it once and avoid any concurrency issues around it.
        Make sure you don't call this until you're done with building your
        manifest!
        """
+        # TODO: we could supply sources in nodes here, for backwards


We might need to do this - interested in feedback on it!

I'm happy to break the existing contract here - using graph.nodes is pretty niche and we can call it out in the release notes / migration guide. I feel great about requiring users to go through graph.sources instead of graph.nodes, as many of the nodes in graph.nodes are going to be poorly constructed at parse-time.

Check out the big disclaimer here for more info: https://docs.getdbt.com/docs/writing-code-in-dbt/jinja-context/graph/

drewbanin

This looks really great! Couple of pieces of feedback from playing around with this locally:

Can we add a warning message if a source patch overrides a source which does not exist? This was actually user I encountered while testing, and it would have been really helpful to see a warning.

~~I see a stack trace when trying to run dbt source snapshot-freshness:~~

User Error

I'll keep testing this, but wanted to send this initial feedback through sooner rather than later. Overall this is very, very good!!

CHANGELOG.md

drewbanin · 2020-04-27T19:37:45Z

core/dbt/contracts/graph/manifest.py


    def build_flat_graph(self):
        """This attribute is used in context.common by each node, so we want to
        only build it once and avoid any concurrency issues around it.
        Make sure you don't call this until you're done with building your
        manifest!
        """
+        # TODO: we could supply sources in nodes here, for backwards


I'm happy to break the existing contract here - using graph.nodes is pretty niche and we can call it out in the release notes / migration guide. I feel great about requiring users to go through graph.sources instead of graph.nodes, as many of the nodes in graph.nodes are going to be poorly constructed at parse-time.

Check out the big disclaimer here for more info: https://docs.getdbt.com/docs/writing-code-in-dbt/jinja-context/graph/

drewbanin · 2020-04-27T19:37:51Z

core/dbt/contracts/graph/manifest.py

        self.flat_graph = {
            'nodes': {
                k: v.to_dict(omit_none=False) for k, v in self.nodes.items()
            },
+            'sources': {


core/dbt/parser/schemas.py

core/dbt/parser/sources.py

core/dbt/task/runnable.py

core/dbt/exceptions.py

beckjake · 2020-04-27T20:52:47Z

Can we add a warning message if a source patch overrides a source which does not exist? This was actually user I encountered while testing, and it would have been really helpful to see a warning.

Yes, definitely, great idea

drewbanin · 2020-04-27T23:07:28Z

I think we've lost sources in the catalog. Can you update the get_catalog impls to also pull source schemas out of the manifest now that they're not reflected in graph.nodes anymore?

The SchemaParser now renders the entire schema.yml except tests/descriptions Snapshot config resolution

…lts/the manifest

add override tests fix some unit tests

Add/modify tests to test native rendering/config application

Handle quoting in render() instead of get_rendered code - avoid stripping the quotes from a string that was returned as jinja

- add paths to source patches, apply them to parsed source definitions - warn on unused source patches - fix error on duplicate source patches - add sources back into the catalog - update changelog

Fix tests accordingly

beckjake · 2020-04-28T17:38:48Z

@drewbanin I believe I've addressed all the PR feedback.

drewbanin

LGTM - ship it!

cla-bot bot added the cla:yes label Apr 24, 2020

beckjake force-pushed the feature/schema-yml-rendering-3 branch from 00515f9 to bd48904 Compare April 24, 2020 17:25

beckjake commented Apr 24, 2020

View reviewed changes

beckjake force-pushed the feature/schema-yml-rendering-3 branch 2 times, most recently from afbbf59 to 6199871 Compare April 24, 2020 17:39

beckjake requested a review from drewbanin April 24, 2020 17:45

beckjake force-pushed the feature/schema-yml-rendering-3 branch from aa93347 to 52cf845 Compare April 24, 2020 20:21

drewbanin reviewed Apr 27, 2020

View reviewed changes

drewbanin mentioned this pull request Apr 27, 2020

Don't render overridden sources twice in docs site #2284

Closed

beckjake force-pushed the feature/schema-yml-rendering-3 branch from fcdca0f to 54c9bc4 Compare April 28, 2020 17:28

Jacob Beck added 11 commits April 28, 2020 11:28

remove docs parse context - not really used

0b7dc32

Give sources a sane FQN, initial schema var rendering stuff

7323ddb

Docs now get the schema.yml parser vars

c19a7d7

The SchemaParser now renders the entire schema.yml except tests/descriptions Snapshot config resolution

add source patches, distinguished by override, store them on run resu…

704e44e

…lts/the manifest

move sources into their own dict

c69f28e

implement source patching

38443cf

Tests

120f0f0

add override tests fix some unit tests

update changelog

eea322e

flip the flag

ff3360a

Add/modify tests to test native rendering/config application

Add file paths to dbt_project.yml when it fails to render

86a35d6

Add some very special quoting rules for rendering

7e85ad9

Handle quoting in render() instead of get_rendered code - avoid stripping the quotes from a string that was returned as jinja

beckjake force-pushed the feature/schema-yml-rendering-3 branch from 54c9bc4 to ed2fb08 Compare April 28, 2020 17:29

Jacob Beck added 2 commits April 28, 2020 11:37

PR feedback

caee941

- add paths to source patches, apply them to parsed source definitions - warn on unused source patches - fix error on duplicate source patches - add sources back into the catalog - update changelog

move catalog nodes entry into sources

d383441

Fix tests accordingly

beckjake force-pushed the feature/schema-yml-rendering-3 branch from ed2fb08 to d383441 Compare April 28, 2020 17:37

Jacob Beck added 2 commits April 28, 2020 11:52

fix catalog unit tests

ee529a5

support windows in source overrides tests

0b82def

beckjake force-pushed the feature/schema-yml-rendering-3 branch from c228f5e to 0b82def Compare April 28, 2020 18:49

drewbanin approved these changes Apr 29, 2020

View reviewed changes

beckjake merged commit 5884f7d into dev/octavius-catto Apr 29, 2020

beckjake deleted the feature/schema-yml-rendering-3 branch April 29, 2020 19:52

jtcohen6 mentioned this pull request May 25, 2020

Threads cannot be set from an environment variable #2486

Closed

5 tasks

jtcohen6 mentioned this pull request Jul 2, 2021

Warnings in Python 3.8 regarding incompatibility #2342

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Schema yaml rendering and source overrides #2357

Schema yaml rendering and source overrides #2357

beckjake commented Apr 24, 2020 •

edited

Loading

beckjake Apr 24, 2020

drewbanin Apr 27, 2020

drewbanin left a comment •

edited

Loading

drewbanin Apr 27, 2020

drewbanin Apr 27, 2020

beckjake commented Apr 27, 2020

drewbanin commented Apr 27, 2020

beckjake commented Apr 28, 2020

drewbanin left a comment

Schema yaml rendering and source overrides #2357

Schema yaml rendering and source overrides #2357

Conversation

beckjake commented Apr 24, 2020 • edited Loading

Description

Checklist

beckjake Apr 24, 2020

Choose a reason for hiding this comment

drewbanin Apr 27, 2020

Choose a reason for hiding this comment

drewbanin left a comment • edited Loading

Choose a reason for hiding this comment

drewbanin Apr 27, 2020

Choose a reason for hiding this comment

drewbanin Apr 27, 2020

Choose a reason for hiding this comment

beckjake commented Apr 27, 2020

drewbanin commented Apr 27, 2020

beckjake commented Apr 28, 2020

drewbanin left a comment

Choose a reason for hiding this comment

beckjake commented Apr 24, 2020 •

edited

Loading

drewbanin left a comment •

edited

Loading