Fix cache swap issue#2332
Conversation
✅ Deploy Preview for astronomer-cosmos ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a cache invalidation bug that occurs when switching between LoadMode.DBT_LS and LoadMode.DBT_MANIFEST with caching enabled. The fix ensures that when a cache of the wrong type is encountered, an empty cache is returned to trigger a cache miss and force recreation of the correct cache type.
Changes:
- Modified
get_dbt_ls_cache()andget_yaml_selectors_cache()to return empty dict when cache data is missing expected keys - Added comprehensive unit tests to verify cache type mismatch handling
- Fixed documentation error that incorrectly referenced "dbt ls cache" in the YAML selectors cache section
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cosmos/dbt/graph.py | Added else blocks to return empty dict when expected cache keys are missing, triggering proper cache misses |
| tests/dbt/test_graph.py | Added two new tests verifying that loading the wrong cache type results in a cache miss |
| docs/configuration/caching.rst | Corrected documentation to reference "yaml selectors cache" instead of "dbt ls cache" in the YAML selectors section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2332 +/- ##
=======================================
Coverage 97.90% 97.90%
=======================================
Files 102 102
Lines 6818 6820 +2
=======================================
+ Hits 6675 6677 +2
Misses 143 143 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tatiana
left a comment
There was a problem hiding this comment.
Thanks a lot, @YourRoyalLinus , for iterating and improving this feature further.
Updated the get cache methods to return an empty cache if they do not find the keys they were expecting. This will lead to a cache miss in the calling function and the recreation of the cache, which is the intended behavior. This will only occur the first time the `LoadMode` of the graph has been changed with caching is enabled. I've added unit tests and completed local QA to confirm behavior. Closes #2330 (cherry picked from commit 4f9f787)
**Enhancements** * Change Snowflake profile mappings to default to four threads by @tatiana in #2374 * Refactor to avoid potential future ``UnboundLocalError`` for ``producer_task`` in ``calculate_tasks_map`` by @rin in #2309 **Bug Fixes** * Fix graph selector when using + selector with ``dbt-loom`` by @award1230 in #2389 * Populate ``compiled_sql`` for ``InvocationMode.SUBPROCESS`` in ``ExecutionMode.WATCHER`` by @pankajkoti in #2319 * Preserve ``extra_context`` for watcher consumer task instances by @pankajkoti in #2381 * Fix watcher: respect ``deferrable=False`` from ``operator_args`` on consumer sensor by @pankajkoti in #2384 * Error handle invalid YAML with ``LoadMode.DBT_MANIFEST`` and ``RenderConfig.selector`` by @jonbillings in #2316 * Fix selecting model when it has the same name as folder by @pankajastro in #2328 * Handle Param Validation errors by @tatiana in #2358 * Fix cache swap issue by @jonbillings in #2332 * Fix leaked semaphore warnings in Airflow 3 by resetting dbt adapters by @pankajkoti in #2335 **Docs** * Document ``ExecutionMode.KUBERNETES`` limitations by @tatiana in #2326 **Others** * Add .airflow-registry.yaml for Airflow Provider Registry by @kaxil in #2387 * Improve test coverage for PR #2307 by @tatiana in #2308 * Address feedback from code review #2389 by @evanvolgas in #2394 Closes: astronomer/oss-integrations-private#333
Enhancements * Change Snowflake profile mappings to default to four threads by @tatiana in #2374 * Refactor to avoid potential future ``UnboundLocalError`` for ``producer_task`` in ``calculate_tasks_map`` by @rin in #2309 Bug Fixes * Fix graph selector when using + selector with ``dbt-loom`` by @award1230 in #2389 * Populate ``compiled_sql`` for ``InvocationMode.SUBPROCESS`` in ``ExecutionMode.WATCHER`` by @pankajkoti in #2319 * Preserve ``extra_context`` for watcher consumer task instances by @pankajkoti in #2381 * Fix watcher: respect ``deferrable=False`` from ``operator_args`` on consumer sensor by @pankajkoti in #2384 * Error handle invalid YAML with ``LoadMode.DBT_MANIFEST`` and ``RenderConfig.selector`` by @YourRoyalLinus in #2316 * Fix selecting model when it has the same name as folder by @pankajastro in #2328 * Handle Param Validation errors by @tatiana in #2358 * Fix cache swap issue by @YourRoyalLinus in #2332 * Fix leaked semaphore warnings in Airflow 3 by resetting dbt adapters by @pankajkoti in #2335 Docs * Document ``ExecutionMode.KUBERNETES`` limitations by @tatiana in #2326 Others * Add .airflow-registry.yaml for Airflow Provider Registry by @kaxil in #2387 * Improve test coverage for PR #2307 by @tatiana in #2308 * Address feedback from code review #2389 by @evanvolgas in #2394 Additional details on this change: - The actual release was done from branch 1.13: https://github.com/astronomer/astronomer-cosmos/releases/tag/astronomer-cosmos-v1.13.1 - This PR aims to update our CHANGELOG, and allow us to give credit to the username that was being questioned by codespell
Description
Updated the get cache methods to return an empty cache if they do not find the keys they were expecting. This will lead to a cache miss in the calling function and the recreation of the cache, which is the intended behavior.
This will only occur the first time the
LoadModeof the graph has been changed with caching is enabled.I've added unit tests and completed local QA to confirm behavior.
Related Issue(s)
closes #2330
Breaking Change?
Checklist