-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add support for env vars in RenderConfig for dbt ls parsing #690
Add support for env vars in RenderConfig for dbt ls parsing #690
Conversation
👷 Deploy Preview for amazing-pothos-a3bca0 processing.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #690 +/- ##
==========================================
+ Coverage 92.93% 92.97% +0.04%
==========================================
Files 55 55
Lines 2292 2306 +14
==========================================
+ Hits 2130 2144 +14
Misses 162 162 ☔ View full report in Codecov by Sentry. |
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.
Hey @jbandoro this change looks great, thank you!
Moving forward, we need to find a way so users don't need to set this type of information both in the RenderConfig
and in the ExecutionConfig
- since, in most cases, they will want the same environment vars defined in both places. A related recent request was to set dbt --vars
in a way they are reused both in RenderConfig
and ExecutionConfig
.
Would ProjectConfig
be a more suitable place for this? Or should we have a new config object, altogether?
@jlaneve would love your thoughts on having a unified interface as well
I agree that having to specify env variables twice, once for parsing and again for execution is not ideal, and I can't think of a reason why they would be different? For example in the example cosmos source dev dag we have: astronomer-cosmos/dev/dags/example_cosmos_sources.py Lines 61 to 83 in 06c4b80
I agree It would be cleaner and less confusing to have env vars specified in one place like |
Bug fixes * Fix running models that use alias while supporting dbt versions by @binhnq94 in astronomer#662 * Make profiles_yml_path optional for ExecutionMode.DOCKER and KUBERNETES by @MrBones757 in astronomer#681 * Prevent overriding dbt profile fields with profile args of type or method by @jbandoro in astronomer#702 * Fix LoadMode.DBT_LS fail when dbt outputs WarnErrorOptions by @adammarples in astronomer#692 * Add support for env vars in RenderConfig for dbt ls parsing by @jbandoro in astronomer#690 * Add support for Kubernetes on_warning_callback by @david-mag in astronomer#673 * Fix ExecutionConfig.dbt_executable_path to use ``default_factory`` by @jbandoro in astronomer#678 Others * Docs fix: example DAG in the README and docs/index by @tatiana in astronomer#705 * Docs improvement: highlight DAG examples in README by @iancmoritz and @jlaneve in astronomer#695 (cherry picked from commit 2878d6a)
[Justin Bandoro](https://www.linkedin.com/in/justin-bandoro-592b14a7/) (@jbandoro) is a Data Engineer at Kevala Inc. He's based in San Francisco (USA) and has been an early adopter of Cosmos, using it regularly at his company. Not only has he been using Cosmos since the early stages, but he has consistently improved Cosmos since January 2023: ![Screenshot 2023-12-04 at 16 28 29](https://github.com/astronomer/astronomer-cosmos/assets/272048/43197938-d1ab-431f-b101-b6026e5cd3ab) Some of his contributions include new features, code quality, documentation and overall improvements. Some examples: * Speed up integration tests in 67% #732 * Prevent override of dbt profile fields #702 * Add support for env vars in `RenderConfig` in #690 * Use symbolic links to run local tasks, avoiding to copy potentially huge dbt project folders in #660 * Improve documentation in #638 * Automated and improved the code complexity checks in #629 * Added `DbtDocsGCSOperator` in #616 * Added support for Python 3.7 in #88 and #214 Additionally, he has been interacting with users in the #airflow-dbt Slack channel in a very collaborative and supportive way. We want to promote him as a Cosmos committer and maintainer for all these, recognising his constant efforts and achievements towards our community. Thank you very much, @jbandoro !
…er#690) Currently, there is a workaround to have environment variables that are required when parsing a dbt project with the dbt ls load mode by setting them with `os.environ` in the DAG file. This is what is currently done in the cosmos dev dag [here](https://github.com/astronomer/astronomer-cosmos/blob/e23a445b30ca391842dae870260cc7ce799d4d5c/dev/dags/example_cosmos_sources.py#L29) since that env var is required for parsing with dbt ls. The problem with setting `os.environ` in that python file is that for the sqlite integration test it was enabling this [test](https://github.com/astronomer/astronomer-cosmos/blob/e23a445b30ca391842dae870260cc7ce799d4d5c/tests/dbt/test_graph.py#L388) to unexpectedly pass (which also requires that env var). This PR adds support for `env_vars` as an argument for `RenderConfig` and sets/unsets the environment variables in a context manager for the dbt ls graph parsing. Closes: astronomer#5 Closes: astronomer#646
Bug fixes * Fix running models that use alias while supporting dbt versions by @binhnq94 in astronomer#662 * Make profiles_yml_path optional for ExecutionMode.DOCKER and KUBERNETES by @MrBones757 in astronomer#681 * Prevent overriding dbt profile fields with profile args of type or method by @jbandoro in astronomer#702 * Fix LoadMode.DBT_LS fail when dbt outputs WarnErrorOptions by @adammarples in astronomer#692 * Add support for env vars in RenderConfig for dbt ls parsing by @jbandoro in astronomer#690 * Add support for Kubernetes on_warning_callback by @david-mag in astronomer#673 * Fix ExecutionConfig.dbt_executable_path to use ``default_factory`` by @jbandoro in astronomer#678 Others * Docs fix: example DAG in the README and docs/index by @tatiana in astronomer#705 * Docs improvement: highlight DAG examples in README by @iancmoritz and @jlaneve in astronomer#695 (cherry picked from commit 2878d6a)
[Justin Bandoro](https://www.linkedin.com/in/justin-bandoro-592b14a7/) (@jbandoro) is a Data Engineer at Kevala Inc. He's based in San Francisco (USA) and has been an early adopter of Cosmos, using it regularly at his company. Not only has he been using Cosmos since the early stages, but he has consistently improved Cosmos since January 2023: ![Screenshot 2023-12-04 at 16 28 29](https://github.com/astronomer/astronomer-cosmos/assets/272048/43197938-d1ab-431f-b101-b6026e5cd3ab) Some of his contributions include new features, code quality, documentation and overall improvements. Some examples: * Speed up integration tests in 67% astronomer#732 * Prevent override of dbt profile fields astronomer#702 * Add support for env vars in `RenderConfig` in astronomer#690 * Use symbolic links to run local tasks, avoiding to copy potentially huge dbt project folders in astronomer#660 * Improve documentation in astronomer#638 * Automated and improved the code complexity checks in astronomer#629 * Added `DbtDocsGCSOperator` in astronomer#616 * Added support for Python 3.7 in astronomer#88 and astronomer#214 Additionally, he has been interacting with users in the #airflow-dbt Slack channel in a very collaborative and supportive way. We want to promote him as a Cosmos committer and maintainer for all these, recognising his constant efforts and achievements towards our community. Thank you very much, @jbandoro !
Description
Currently there is a workaround to have environment variables that are required when parsing a dbt project with the dbt ls load mode by setting them with
os.environ
in the DAG file.This is what is currently done in the cosmos dev dag here since that env var is required for parsing with dbt ls. The problem with setting
os.environ
in that python file is that for the sqlite integration test it was enabling this test to unexpectedly pass (which also requires that env var).This PR adds support for
env_vars
as an argument forRenderConfig
and sets/unsets the environment variables in a context manager for the dbt ls graph parsing.Related Issue(s)
closes #550 and closes #646
Breaking Change?
None
Checklist