Skip to content

Bugfix operator_args override configuration#1558

Merged
tatiana merged 3 commits into
astronomer:mainfrom
ghjklw:bugfix-override-operator-args
Feb 27, 2025
Merged

Bugfix operator_args override configuration#1558
tatiana merged 3 commits into
astronomer:mainfrom
ghjklw:bugfix-override-operator-args

Conversation

@ghjklw
Copy link
Copy Markdown
Contributor

@ghjklw ghjklw commented Feb 24, 2025

Description

The function cosmos.converter.override_configuration had a small logical issue: the condition to override install_deps
could never be reached. This seems to be the root cause of #1557

Related Issue(s)

Closes #1557

Breaking Change?

No

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

@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Feb 24, 2025
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 24, 2025 •
edited
Loading

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

Name Link
🔨 Latest commit b064379
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/67c074461239df0008154667

@dosubot dosubot Bot added the area:config Related to configuration, like YAML files, environment variables, or executer configuration label Feb 24, 2025
@tatiana tatiana added this to the Cosmos 1.9.1 milestone Feb 24, 2025
tatiana
tatiana reviewed Feb 24, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this fix, @ghjklw !
Is there any chance you could add a unit test?

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.34%. Comparing base (7df2fde) to head (b064379).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1558      +/-   ##
==========================================
+ Coverage   97.30%   97.34%   +0.04%     
==========================================
  Files          80       80              
  Lines        4902     4902              
==========================================
+ Hits         4770     4772       +2     
+ Misses        132      130       -2

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

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Feb 25, 2025
@ghjklw
Copy link
Copy Markdown
Contributor Author

ghjklw commented Feb 25, 2025

Thanks a lot for this fix, @ghjklw ! Is there any chance you could add a unit test?

Done :)

tatiana
tatiana approved these changes Feb 26, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this and adding this very thorough test, @ghjklw ! We'll merge once our CI is stable (unrelated to your change).

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Feb 26, 2025
@tatiana tatiana changed the title Bugfix operator_args override configuration Bugfix operator_args override configuration Feb 27, 2025
@tatiana tatiana merged commit dfbef5c into astronomer:main Feb 27, 2025
pankajkoti pushed a commit that referenced this pull request Mar 13, 2025
@ghjklw @pankajkoti
Bugfix operator_args override configuration (#1558)
167f23b 
The function `cosmos.converter.override_configuration` had a small
logical issue: the condition to override `install_deps`
could never be reached. This seems to be the root cause of #1557

Closes #1557

(cherry picked from commit dfbef5c)
@pankajkoti pankajkoti mentioned this pull request Mar 13, 2025
Closed
@tatiana tatiana mentioned this pull request Mar 13, 2025
Merged
tatiana added a commit that referenced this pull request Mar 17, 2025
@tatiana @pankajkoti
Release 1.9.1 changelog (#1612)
8f6fb24 
Bug Fixes

* Fix import error in dbt bigquery adapter mock for ``dbt-bigquery<1.8``
for ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti in #1548
* Fix ``operator_args`` override configuration by @ghjklw in #1558
* Fix missing ``install_dbt_deps`` in ``ProjectConfig`` ``__init__``
method by @ghjklw in #1556
* Fix dbt project parsing ``dbt_vars`` behavior passed via
``operator_args`` by @AlexandrKhabarov in #1543
* Avoid reading the connection during DAG parsing of the async BigQuery
operator by @joppevos in #1582
* Fix: Workaround to incorrectly raised ``gcsfs.retry.HttpError``
(Invalid Credentials, 401) by @tatiana in #1598
* Fix the async execution mode read sql files for dbt packages by
@pankajastro in #1588
* Improve BQ async error handling by @tatiana in #1597
* Fix path selector when ``manifest.json`` is created using MS Windows
by @tatiana in #1601
* Fix log that prints 'Total filtered nodes' by @tatiana in #1603
* Fix select behaviour using ``LoadMode.MANIFEST`` and a path with star
by @tatiana in #1602
* Support ``on_warning_callback`` with ``TestBehavior.BUILD`` and
``ExecutionMode.LOCAL`` by @corsettigyg in #1571
* Fix ``DbtRunLocalOperator.partial()`` support by @tatiana @ashb in
#1609
* fix: ``container_name`` is null for ecs integration by @nicor88 in
#1592

Docs

* Improve MWAA getting-started docs by removing unused imports by
@jx2lee in #1562

Others

* Disable ``example_cosmos_dbt_build.py`` DAG in CI by @pankajastro in
#1567
* Upgrade GitHub Actions Ubuntu version by @tatiana in #1561
* Update GitHub bug issue template by @pankajastro in #1586
* Enable DAG ``example_cosmos_dbt_build.py`` in CI by @pankajastro in
#1573
* Run async DAG in DAG without setup/teardown task by @pankajastro in
#1599
* Add test case that fully covers recent select issue by @tatiana in
#1604
* Add CI job to test multiple dbt versions for the async DAG by
@pankajkoti in #1535
* Improve unit tests speed from 89s to 14s by @tatiana in #1600
* Pre-commit updates: #1560, #1583, #1596


Closes: #1550

Mergeable version of
#1607

Co-authored-by: Pankaj Singh
<98807258+pankajastro@users.noreply.github.com>
Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tatiana tatiana tatiana approved these changes

@pankajastro pankajastro pankajastro approved these changes

Assignees

No one assigned

Labels

area:config Related to configuration, like YAML files, environment variables, or executer configuration lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

Cosmos 1.9.1

Development

Successfully merging this pull request may close these issues.

[Bug] unexpected error message when setting ProjectConfig.install_dbt_deps to False

3 participants

@ghjklw @tatiana @pankajastro