[#3885] Skip partial parsing if project env vars change #4212

gshank · 2021-11-05T00:07:51Z

resolves #3885

Description

This pull request captures the env vars when the dbt_project.yml file is rendered and saves a checksum of them in the manifest.state_check. When partial parsing is attempted it will compare the checksums and skip partial parsing if they have changed.

Profile env vars have also been handled. Note: only the specified profile is actually rendered.

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

jtcohen6

Just spent some time playing around with these changes. Looking good so far!

jtcohen6

Working great in local testing. Good to go from my perspective.

Let's make sure someone is able to give this PR + #4162 a quick code review on Monday morning, so that we can have both in rc1.

jtcohen6 · 2021-11-05T21:20:22Z

core/dbt/parser/manifest.py

+    proj_env_vars_changed = '09_project_env_vars_changed'
+    prof_env_vars_changed = '10_profile_env_vars_changed'


emmyoop · 2021-11-08T15:13:05Z

core/dbt/config/project.py

+        # There will never be any project_env_vars when it's first created
+        project_env_vars: Dict[str, Any] = {}


Is this true? With the new interactive init you can technically put in env_vars. Either way they're "changing" since they're new so not sure how much it matters.

Yes. The project_env_vars only come from rendering, which happens after the object is created. So whether or not env vars exist before the dbt code starts up doesn't matter.

emmyoop

LGTM but I'd be happier is someone else took a look too.

emmyoop · 2021-11-08T15:19:55Z

core/dbt/parser/manifest.py

+            logger.info("Unable to do partial parsing because env vars "
+                        "used in dbt_project.yml have changed")
+            valid = False
+            reparse_reason = ReparseReason.proj_env_vars_changed
+        if self.manifest.state_check.profile_env_vars_hash != \
+                manifest.state_check.profile_env_vars_hash:
+            logger.info("Unable to do partial parsing because env vars "
+                        "used in profiles.yml have changed")


Depending on what goes in first (this or #4055) , this will need to be updated to the new struct logging as well as anywhere else logging is happening. Nothing to do now - more of a reminder for later.

I think we should have a walkthrough next week since everything is all crunchy right now.

* [#3885] Skip partial parsing if project env vars change * Support env_vars in the profile automatic commit by git-black, original commits: 3e9da06

cla-bot bot added the cla:yes label Nov 5, 2021

jtcohen6 reviewed Nov 5, 2021

View reviewed changes

gshank force-pushed the pp_project_env_vars branch from 166aaff to 4beaa25 Compare November 5, 2021 17:36

gshank requested review from jtcohen6, nathaniel-may, emmyoop and leahwicz November 5, 2021 17:40

jtcohen6 reviewed Nov 5, 2021

View reviewed changes

emmyoop reviewed Nov 8, 2021

View reviewed changes

emmyoop approved these changes Nov 8, 2021

View reviewed changes

Base automatically changed from pp_schema_env_vars to main November 8, 2021 16:28

gshank added 2 commits November 8, 2021 11:35

[#3885] Skip partial parsing if project env vars change

8c4b97d

Support env_vars in the profile

c449da1

gshank force-pushed the pp_project_env_vars branch from 4beaa25 to c449da1 Compare November 8, 2021 16:35

gshank merged commit 3e9da06 into main Nov 8, 2021

gshank deleted the pp_project_env_vars branch November 8, 2021 16:51

iknox-fa pushed a commit that referenced this pull request Feb 8, 2022

[#3885] Skip partial parsing if project env vars change (#4212)

b2d411b

* [#3885] Skip partial parsing if project env vars change * Support env_vars in the profile automatic commit by git-black, original commits: 3e9da06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#3885] Skip partial parsing if project env vars change #4212

[#3885] Skip partial parsing if project env vars change #4212

gshank commented Nov 5, 2021 •

edited

Loading

jtcohen6 left a comment •

edited

Loading

jtcohen6 left a comment

jtcohen6 Nov 5, 2021

emmyoop Nov 8, 2021 •

edited

Loading

gshank Nov 8, 2021

emmyoop left a comment

emmyoop Nov 8, 2021

gshank Nov 8, 2021

		proj_env_vars_changed = '09_project_env_vars_changed'
		prof_env_vars_changed = '10_profile_env_vars_changed'

		# There will never be any project_env_vars when it's first created
		project_env_vars: Dict[str, Any] = {}

[#3885] Skip partial parsing if project env vars change #4212

[#3885] Skip partial parsing if project env vars change #4212

Conversation

gshank commented Nov 5, 2021 • edited Loading

Description

Checklist

jtcohen6 left a comment • edited Loading

Choose a reason for hiding this comment

jtcohen6 left a comment

Choose a reason for hiding this comment

jtcohen6 Nov 5, 2021

Choose a reason for hiding this comment

emmyoop Nov 8, 2021 • edited Loading

Choose a reason for hiding this comment

gshank Nov 8, 2021

Choose a reason for hiding this comment

emmyoop left a comment

Choose a reason for hiding this comment

emmyoop Nov 8, 2021

Choose a reason for hiding this comment

gshank Nov 8, 2021

Choose a reason for hiding this comment

gshank commented Nov 5, 2021 •

edited

Loading

jtcohen6 left a comment •

edited

Loading

emmyoop Nov 8, 2021 •

edited

Loading