[Bug] - [#1835: Refactor flag resolution logic for dbt dependencies in AbstractDbtLocalBase] by gavin-burns-US · Pull Request #1836 · astronomer/astronomer-cosmos

gavin-burns-US · 2025-06-26T11:32:48Z

User description

#1835: Refactor flag resolution logic for dbt dependencies in AbstractDbtLocalBase

Description

This PR refactors the AbstractDbtLocalBase class to align the flag resolution logic for copy_dbt_packages, install_dbt_deps, and the deprecated install_deps parameter with the standard pattern used in other Cosmos operators. It ensures consistent handling of these flags, corrects defaulting behavior for copy_dbt_packages, and adds or updates unit tests verifying all logic branches and deprecation warnings.

Related Issue(s)

Closes #1835: Refactor flag resolution logic for dbt dependencies in AbstractDbtLocalBase

Breaking Change?

No breaking changes are introduced. The changes maintain backward compatibility by supporting the deprecated install_deps flag with a warning.

Checklist

I have made corresponding changes to the documentation (if required)
I have added tests that prove my fix is effective or that my feature works

PR Type

Bug fix, Enhancement

Description

Deprecate install_deps parameter in favor of install_dbt_deps
Add comprehensive flag resolution logic with proper precedence
Implement full support for copy_dbt_packages flag handling
Add extensive unit tests for all flag combinations

Changes walkthrough 📝

Relevant files

Enhancement

local.py `Refactor flag resolution and deprecate install_deps` cosmos/operators/local.py Add deprecation warnings for `install_deps` parameter usage Implement flag precedence logic: kwarg > operator_args > default Add full support for `copy_dbt_packages` flag resolution Refactor `__init__` method to handle multiple flag sources consistently	+63/-12

Tests

test_local.py `Add comprehensive tests for flag resolution` `tests/operators/test_local.py` Add comprehensive test coverage for flag resolution logic Test deprecation warnings for legacy `install_deps` parameter Verify precedence handling for all supported flags Add parameterized tests for various flag combinations	+112/-5

Reviewer's Guide

This PR refactors the AbstractDbtLocalBase operator to centralize and standardize how install_dbt_deps (and its deprecated alias install_deps) and copy_dbt_packages flags are handled via a new operator_args mechanism, and augments the test suite with comprehensive, parameterized coverage for all resolution paths and deprecation warnings.

Class diagram for AbstractDbtLocalBase flag resolution refactor

classDiagram
    class AbstractDbtLocalBase {
        +dict operator_args
        +bool install_dbt_deps
        +bool install_deps
        +bool copy_dbt_packages
        +__init__(...)
    }
    AbstractDbtLocalBase --|> AbstractDbtBase
    class AbstractDbtBase {
    }
    class ProfileConfig {
    }
    AbstractDbtLocalBase o-- ProfileConfig : profile_config

Class diagram for operator_args influence on AbstractDbtLocalBase

classDiagram
    class AbstractDbtLocalBase {
        +dict operator_args
        +__init__(..., operator_args: dict[str, Any] | None = None, ...)
    }
    AbstractDbtLocalBase : operator_args influences install_dbt_deps
    AbstractDbtLocalBase : operator_args influences copy_dbt_packages

File-Level Changes

Change	Details	Files
Refactor flag resolution in AbstractDbtLocalBase to unify install_dbt_deps, handle deprecated install_deps, and copy_dbt_packages	Introduce and store operator_args dict in init Emit DeprecationWarning whenever install_deps is passed Resolve install_dbt_deps by priority: explicit kwarg > operator_args > deprecated flags > default Override deps install to False when no dependencies file is present Alias install_deps to install_dbt_deps Implement copy_dbt_packages resolution: operator_args > explicit kwarg > global default	`cosmos/operators/local.py`
Extend unit tests to cover all flag resolution scenarios and deprecation warnings	Import default_copy_dbt_packages for default-value tests Add parameterized tests for install_dbt_deps resolution with and without legacy install_deps flags Add tests asserting deprecation warnings for deprecated install_deps usage Add parameterized tests for copy_dbt_packages resolution against operator_args, kwargs, and default Update existing empty-directory test to mock dependencies file and assert both install flags	`tests/operators/test_local.py`

Enhancements to Configuration and Deprecation Handling

Added install_dbt_deps as a replacement for the deprecated install_deps parameter.
When install_deps is used, a deprecation warning is emitted. The logic for resolving dependency installation flags prioritizes install_dbt_deps over install_deps.
See deprecation and resolution logic
Introduced the operator_args parameter, allowing a dictionary of arguments to be passed to the dbt command for greater flexibility.
See operator_args handling
Refactored imports for readability.
See import refactor

Improvements to Dependency Handling

Enhanced logic to automatically disable dependency installation (install_dbt_deps) when no dependencies file is found in the project directory.
Relevant logic

Test Coverage Enhancements

New and expanded tests verify the resolution logic for install_dbt_deps, proper emission of deprecation warnings, and correct behavior for the copy_dbt_packages parameter (covering default and explicit values).
See new and expanded tests
Tests ensure compatibility with deprecated install_deps and validate logic for scenarios where no dependencies file is present.
See test logic
Test imports updated to include warnings and the default_copy_dbt_packages setting for completeness.
See updated test imports

These changes improve the maintainability and usability of the cosmos module while ensuring backward compatibility and robust test coverage.
View the full PR diff here.

netlify · 2025-06-26T11:33:18Z

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name	Link
🔨 Latest commit	`c637ba5`
🔍 Latest deploy log	https://app.netlify.com/projects/sunny-pastelito-5ecb04/deploys/685f28cab4c12b0008b51d7e

Copilot

Pull Request Overview

This PR refactors the installation flag resolution for dbt dependencies in the local operator, ensuring alignment with the standard pattern used across Cosmos operators while maintaining backward compatibility for the deprecated install_deps flag.

Updates the flag resolution logic for install_dbt_deps and the deprecated install_deps parameter.
Adjusts the copy_dbt_packages resolution and updates related unit tests to verify behavior.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/operators/test_local.py	Added tests to cover both new and deprecated flag behaviors.
cosmos/operators/local.py	Refactored init parameters, deprecation warnings, and flag resolution logic.

Comments suppressed due to low confidence (1)

cosmos/operators/local.py:154

[nitpick] Align the parameter documentation order with the updated function signature to enhance clarity in the API documentation.

    :param operator_args: A dictionary of arguments to pass to the dbt command

gavin-burns-US · 2025-06-26T11:35:15Z

@tatiana will look to cleanup my safe additional handling of depr install_deps along with install_dbt_deps -- operator_args as well but any chance you could take a look at this?

at my company we were doing POC of cosmos for a microbatch dbt cloud proj job due to high costs from charge per model build. When we built a workaround for dbt_project.yml based on_run_end configuration it required the use of DbtRunOperationLocalOperator post taskgroup completion. Noticed that no matter the args passed packages/dependencies were still being installed. We are utilizing copy_dbt_packages to get this microbatch based proj as fast as poss.

appreciate it

tatiana

Thanks for the improvement on Cosmos, @gavin-burns-US !

I gave broader feedback on the original issue description:
#1835 (comment)

While we're not ahead with the proposed change of exposing operator_args in Cosmos operators, we're pleased with the refactor that renames dbt_deps to install_dbt_deps and with your improvement in raising the warning to guide users through the migration process.

Your original issue and the PR description mention a bug, but it is not clear which was he code that worked before Cosmos 1.10, and stopped working with Cosmos 1.10 after the refactor #1670. I suggest you have a separate PR just for the bug and the fix, which is unrelated to the refactor of replacing install_deps with install_dbt_deps.

tatiana · 2025-06-26T12:53:22Z

+    :param install_dbt_deps: If true, install dependencies before running the command.
    :param install_deps (deprecated): If true, install dependencies before running the command
    :param copy_dbt_packages: If true, copy pre-existing `dbt_packages` (before running dbt deps)
+    :param operator_args: A dictionary of arguments to pass to the dbt command


@gavin-burns-US As explained in:
#1835 (comment)
operator_args is exposed in DbtDag and DbtTaskGroup, but it is intentionally flattened in the interface of Cosmos operators.

this makes total sense - operator_args only beneficial at a group/rolled up hierarchy of children tasks whether that be taskgroup,dag, etc...
sitting here laughing at how redundant it would at individual operator grain

we are aligned

as you stated in one of the comments ..at that point we are just serving **kwargs via the operator_args docs users can use if they please via **operator_args

okay i will tidy all of this up over next few days + loop in those execution logs / dag src code + dbt project structure

tatiana · 2025-06-26T13:03:07Z

    :param profile_args: Arguments to pass to the profile. See
        :py:class:`cosmos.providers.dbt.core.profiles.BaseProfileMapping`.
    :param profile_config: ProfileConfig Object
+    :param install_dbt_deps: If true, install dependencies before running the command.


I like the idea of introducing install_dbt_deps and deprecating install_deps. It makes the interface consistent with ProjectConfig.install_dbt_deps. Please, if possible, create a separate PR or repurpose this PR only to have this change, and also update the documentation

will do - thanks for the quick review & explanation behind omitting operator_args from these operators. will follow up tomorrow

tatiana · 2025-06-26T13:05:08Z

+        if "install_deps" in self.operator_args:
+            warnings.warn(
+                message="'install_deps' is deprecated. Use 'install_dbt_deps' instead.",
+                category=DeprecationWarning,
+                stacklevel=2,
+            )


We should not check this because this operator will not have an argument named operator_args.

tatiana · 2025-06-26T13:06:58Z

+        deps_flag = None
+        if install_dbt_deps is not None:
+            deps_flag = install_dbt_deps
+        elif "install_dbt_deps" in self.operator_args:


We can remove this

tatiana · 2025-06-26T13:12:40Z

+        # Emit deprecation warnings if install_deps is supplied in any way (regardless of value)
+        if "install_deps" in kwargs:
+            warnings.warn(
+                message="'install_deps' is deprecated. Use 'install_dbt_deps' instead.",


You can also mention that the install_deps argument will be removed in Cosmos 2.

codecov · 2025-06-26T13:59:21Z

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.00%. Comparing base (791b241) to head (1877a53).

Files with missing lines	Patch %	Lines
cosmos/operators/local.py	96.42%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1836      +/-   ##
==========================================
- Coverage   98.01%   98.00%   -0.01%     
==========================================
  Files          85       85              
  Lines        5290     5316      +26     
==========================================
+ Hits         5185     5210      +25     
- Misses        105      106       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gavin-burns-US · 2025-06-27T01:17:51Z

Thanks for the improvement on Cosmos, @gavin-burns-US !

I gave broader feedback on the original issue description:

#1835 (comment)

While we're not ahead with the proposed change of exposing operator_args in Cosmos operators, we're pleased with the refactor that renames dbt_deps to install_dbt_deps and with your improvement in raising the warning to guide users through the migration process.

Your original issue and the PR description mention a bug, but it is not clear which was he code that worked before Cosmos 1.10, and stopped working with Cosmos 1.10 after the refactor #1670. I suggest you have a separate PR just for the bug and the fix, which is unrelated to the refactor of replacing install_deps with install_dbt_deps.

sounds like a plan - will actually share dag , dbt dir structure , etc tomorrow so you can reproduce !

Align AbstractDbtLocalBase flag resolution and deprecation with core operators - Refactored AbstractDbtLocalBase.__init__ to accept `install_dbt_deps`, `install_deps` (deprecated), and `copy_dbt_packages` as both keyword arguments and within `operator_args`. - Added DeprecationWarning when legacy `install_deps` is used, matching deprecation patterns in Docker and K8s operators. - Implemented flag precedence: explicit kwarg > operator_args > default. - This brings local operator behavior in line with other Cosmos operators for a consistent user experience. See #1832 for context and acceptance criteria. Update test_local.py test(local): Add unit tests for AbstractDbtLocalBase flag resolution and deprecation warnings - Add tests for `install_dbt_deps`, legacy `install_deps`, and `copy_dbt_packages` flag resolution in AbstractDbtLocalBase. - Verify correct precedence: explicit kwarg > operator_args > default. - Ensure DeprecationWarning is triggered when using `install_deps`. - Covers all logic branches added in local.py refactor. Relates to @astronomer/astronomer-cosmos#1832 🎨 [pre-commit.ci] Auto format from pre-commit.com hooks Update test_local.py add missing `macro_name` arg to tests Update local.py remove multiple type annotations for self.install_dbt_deps within the same __init__ block—only the first assignment should have the type hint, subsequent assignments just assign the value. feat(local): deprecate `install_deps`, add full `copy_dbt_packages` support, and align precedence logic - Deprecate `install_deps` in favor of `install_dbt_deps` for local operators, emitting a `DeprecationWarning` when the old flag is used (as kwarg or in `operator_args`). - Accept both flags as explicit kwargs or via `operator_args`, with precedence: keyword > operator_args > default. - Make `self.install_deps` an internal alias for `self.install_dbt_deps` for compatibility. - Apply the same precedence logic to `copy_dbt_packages`, supporting override via explicit kwarg or `operator_args`, defaulting to `settings.default_copy_dbt_packages`. - Do not apply the dependency file existence check to the flag itself; only check for files when actually running deps. - Brings local operator behavior in line with Docker/K8s operators and addresses #1831. 🎨 [pre-commit.ci] Auto format from pre-commit.com hooks Update local.py Fix: Only set install_deps True if dependency files exist in project - Refines install_deps and install_dbt_deps logic in AbstractDbtLocalBase to check for dbt dependency files. - Now, install_deps is set to True only if the flag is set and dependency files are present in the project directory. - Ensures compatibility with test expectations and avoids unnecessary dbt deps runs in empty projects. Update local.py fix removed import Update local.py test fail fix ``` =========================== short test summary info ============================ FAILED tests/operators/test_local.py::test_install_dbt_deps_resolution[op_args1-kw1-False] - assert False == True FAILED tests/operators/test_local.py::test_install_dbt_deps_resolution[op_args3-kw3-False] - assert False == True FAILED tests/operators/test_local.py::test_install_dbt_deps_resolution[op_args4-kw4-True] - assert False is True + where False = <Task(DbtRunOperationLocalOperator): macro>.install_dbt_deps = 3 failed, 866 passed, 4 skipped, 96 deselected, 2 xfailed, 5 warnings in 24.29s = Error: Process completed with exit code 1. ``` Update test_local.py fix tests params Update test_local.py update 🎨 [pre-commit.ci] Auto format from pre-commit.com hooks fix: ensure dependencies are always checked for non-empty files in AbstractDbtLocalBase Signed-off-by: Gavin Burns <gavin.burns@leovegas.com> fix: update deprecation warnings for install_deps in AbstractDbtLocalBase Signed-off-by: Gavin Burns <gavin.burns@leovegas.com> fix: update deprecation warning condition for install_deps in AbstractDbtLocalBase Signed-off-by: Gavin Burns <gavin.burns@leovegas.com> test: refactor and add deprecation warnings for install_deps in DbtRunOperationLocalOperator tests Signed-off-by: Gavin Burns <gavin.burns@leovegas.com> 🎨 [pre-commit.ci] Auto format from pre-commit.com hooks fix: update deprecation warnings for install_deps in AbstractDbtLocalBase Signed-off-by: Gavin Burns <gavin.burns@leovegas.com> Update local.py mypy type issue add test for copy_dbt_packages flag resolution logic 🎨 [pre-commit.ci] Auto format from pre-commit.com hooks Prioritizes operator args for package copy Adjusts the precedence logic for `copy_dbt_packages` to ensure values specified within `operator_args` take priority over explicitly passed keyword arguments. This aligns with expected configuration override patterns. Signed-off-by: Gavin Burns <gavin.burns@leovegas.com> Update default_copy_dbt_packages usage in test_install_dbt_deps_resolution test Signed-off-by: Gavin Burns <gavin.burns@leovegas.com>

gavin-burns-US · 2025-06-28T00:03:06Z

Thanks for the improvement on Cosmos, @gavin-burns-US !

I gave broader feedback on the original issue description: #1835 (comment)

While we're not ahead with the proposed change of exposing operator_args in Cosmos operators, we're pleased with the refactor that renames dbt_deps to install_dbt_deps and with your improvement in raising the warning to guide users through the migration process.

Your original issue and the PR description mention a bug, but it is not clear which was he code that worked before Cosmos 1.10, and stopped working with Cosmos 1.10 after the refactor #1670. I suggest you have a separate PR just for the bug and the fix, which is unrelated to the refactor of replacing install_deps with install_dbt_deps.

@github resolve

Copilot AI review requested due to automatic review settings June 26, 2025 11:32

gavin-burns-US had a problem deploying to external June 26, 2025 11:32 — with GitHub Actions Error

Copilot AI reviewed Jun 26, 2025

View reviewed changes

Comment thread cosmos/operators/local.py

gavin-burns-US changed the title ~~Update local.py~~ [Bug] [#1835] Local Operator Flag Handling & Abstract Operator Influence - install_deps deprecation + allow copy_dbt_packages + operator_args Jun 26, 2025

gavin-burns-US had a problem deploying to external June 26, 2025 11:37 — with GitHub Actions Error

pre-commit-ci Bot had a problem deploying to external June 26, 2025 11:39 Error

gavin-burns-US had a problem deploying to external June 26, 2025 11:43 — with GitHub Actions Error

gavin-burns-US had a problem deploying to external June 26, 2025 11:49 — with GitHub Actions Error

gavin-burns-US temporarily deployed to external June 26, 2025 11:51 — with GitHub Actions Inactive

gavin-burns-US mentioned this pull request Jun 26, 2025

[Bug] Local Operator Flag Handling & Abstract Operator Influence - install_deps deprecation + allow copy_dbt_packages + operator_args #1835

Closed

1 task

gavin-burns-US changed the title ~~[Bug] - Local Operator Flag Handling & Abstract Operator Influence - install_deps deprecation + allow copy_dbt_packages + operator_args - [#1835]~~ [Bug] - [#1835: Refactor flag resolution logic for dbt dependencies in AbstractDbtLocalBase] Jun 26, 2025

tatiana reviewed Jun 26, 2025

View reviewed changes

gavin-burns-US had a problem deploying to external June 27, 2025 23:27 — with GitHub Actions Failure

gavin-burns-US closed this by deleting the head repository Jun 28, 2025

This was referenced Jun 28, 2025

[Enhancement] Deprecate install_deps in favor of install_dbt_deps for local operators #1853

Closed

[Enhancement] Deprecate install_deps in favor of install_dbt_deps for local operators #1854

Closed

Conversation

gavin-burns-US commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Description

Related Issue(s)

Breaking Change?

Checklist

PR Type

Description

Changes walkthrough 📝

Reviewer's Guide

Class diagram for AbstractDbtLocalBase flag resolution refactor

Class diagram for operator_args influence on AbstractDbtLocalBase

File-Level Changes

Enhancements to Configuration and Deprecation Handling

Improvements to Dependency Handling

Test Coverage Enhancements

Uh oh!

netlify Bot commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

gavin-burns-US commented Jun 26, 2025

Uh oh!

tatiana left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jun 26, 2025

Codecov Report

Uh oh!

gavin-burns-US commented Jun 27, 2025

Uh oh!

gavin-burns-US commented Jun 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gavin-burns-US commented Jun 26, 2025 •

edited

Loading

netlify Bot commented Jun 26, 2025 •

edited

Loading

tatiana left a comment •

edited

Loading