Fix custom "+tag:some_tag" selector issue related to tests tag inheritance #1466

tatiana · 2025-01-14T13:17:51Z

The selector method _should_include_node changes test tasks to inherit tags from their parent nodes. While this behaviour is acceptable and desirable in some cases, it can cause problems using graph selectors with tags.

This PR improves the test coverage, narrows down the problem and fixes the problem reported by Astronomer customers. More details below.

A user reported that they see the correct DbtDag when using Cosmos 1.8.1 with:

LoadMode.DBT_LS
RenderConfig(selector="accounts_marts")

Where the selector accounts_marts is defined as:

    - name: accounts_marts
      description: Run Accounts models
      definition:
        intersection:
          - '+tag:accounts'
          - '+tag:datamart'
          - '+tag:stratus'

The expected behaviour includes:

164 Airflow tasks
152 Local run tasks
12 Snapshot tasks

However, when they attempt to run the same DbtDag using:

LoadMode.DBT_MANIFEST
RenderConfig(select=["+tag:accounts,+tag:datamart,+tag:stratus"])

Their DbtDag seems to have the wrong subset of nodes.

They reported:

197 Airflow tasks
183 Local run tasks
14 Snapshot tasks

This pull request aims to reproduce and fix this issue.

netlify · 2025-01-14T13:18:07Z

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

Name	Link
🔨 Latest commit	`c8cbfe9`
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/678f7d1a80b01b00083e8534

cloudflare-workers-and-pages · 2025-01-14T13:32:26Z

Deploying astronomer-cosmos with Cloudflare Pages

Latest commit:	`c8cbfe9`
Status:	✅ Deploy successful!
Preview URL:	https://cf3b5dcd.astronomer-cosmos.pages.dev
Branch Preview URL:	https://select-intersection-graphs.astronomer-cosmos.pages.dev

View logs

codecov · 2025-01-14T13:52:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.57%. Comparing base (387558f) to head (c8cbfe9).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1466      +/-   ##
==========================================
- Coverage   96.95%   96.57%   -0.39%     
==========================================
  Files          73       73              
  Lines        4371     4374       +3     
==========================================
- Hits         4238     4224      -14     
- Misses        133      150      +17

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

…selectors

pankajkoti

Looks great to me! Fantastic breakdown and analysis in identifying and addressing this fix 👏🏽

cosmos/dbt/selector.py

@tatiana

Breaking changes * When using ``LoadMode.DBT_LS``, Cosmos will now attempt to use the ``dbtRunner`` as opposed to subprocess to run ``dbt ls``. While this represents significant performance improvements (half the vCPU usage and some memory consumption improvement), this may not work in scenarios where users had multiple Python virtual environments to manage different versions of dbt and its adaptors. In those cases, please, set ``RenderConfig(invocation_mode=InvocationMode.SUBPROCESS)`` to have the same behaviour Cosmos had in previous versions. Additional information `here <https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls>`_ and `here <https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#how-to-run-dbt-ls-invocation-mode>`_. Features * Use ``dbtRunner`` in the DAG Processor when using ``LoadMode.DBT_LS`` if ``dbt-core`` is available by @tatiana in #1484. Additional information `here <https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls>`_. * Allow users to opt-out of ``dbtRunner`` during DAG parsing with ``InvocationMode.SUBPROCESS`` by @tatiana in #1495. Check out the `documentation <https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#how-to-run-dbt-ls-invocation-mode>`_. * Add structure to support multiple db for async operator execution by @pankajastro in #1483 * Support overriding the ``profile_config`` per dbt node or folder using config by @tatiana in #1492. More information `here <https://astronomer.github.io/astronomer-cosmos/profiles/#profile-customise-per-node>`_. * Create and run accurate SQL statements when using ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti, @tatiana and @pankajastro in #1474 * Add AWS ECS task run execution mode by @CarlosGitto and @aoelvp94 in #1507 * Add support for running ``DbtSourceOperator`` individually by @victormacaubas in #1510 * Add setup task for async executions by @pankajastro in #1518 * Add teardown task for async executions by @pankajastro in #1529 * Add ``ProjectConfig.install_dbt_deps`` & change operator ``install_deps=True`` as default by @tatiana in #1521 * Extend Virtualenv operator and mock dbt adapters for setup & teardown tasks in ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti, @tatiana and @pankajastro in #1544 Bug Fixes * Fix select complex intersection of three tag-based graph selectors by @tatiana in #1466 * Fix custom selector behaviour when the model name contains periods by @yakovlevvs and @60098727 in #1499 * Filter dbt and non-dbt kwargs correctly for async operator by @pankajastro in #1526 Enhancement * Fix OpenLineage deprecation warning by @CorsettiS in #1449 * Move ``DbtRunner`` related functions into ``dbt/runner.py`` module by @tatiana in #1480 * Add ``on_warning_callback`` to ``DbtSourceKubernetesOperator`` and refactor previous operators by @LuigiCerone in #1501 * Gracefully error when users set incompatible ``RenderConfig.dbt_deps`` and ``operator_args`` ``install_deps`` by @tatiana in #1505 * Store compiled SQL as template field for ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti in #1534 Docs * Improve ``RenderConfig`` arguments documentation by @tatiana in #1514 * Improve callback documentation by @tatiana in #1516 * Improve partial parsing docs by @tatiana in #1520 * Fix typo in selecting & excluding docs by @pankajastro in #1523 * Document ``async_py_requirements`` added in ``ExecutionConfig`` for ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti in #1545 Others * Ignore dbt package tests when running Cosmos tests by @tatiana in #1502 * Refactor to consolidate async dbt adapter code by @pankajkoti in #1509 * Log elapsed time for sql file(s) upload/download by @pankajastro in #1536 * Remove the fallback operator for async task by @pankajastro in #1538 * GitHub Actions Dependabot: #1487 * Pre-commit updates: #1473, #1493, #1503, #1531

@tatiana

Breaking changes * When using ``LoadMode.DBT_LS``, Cosmos will now attempt to use the ``dbtRunner`` as opposed to subprocess to run ``dbt ls``. While this represents significant performance improvements (half the vCPU usage and some memory consumption improvement), this may not work in scenarios where users had multiple Python virtual environments to manage different versions of dbt and its adaptors. In those cases, please, set ``RenderConfig(invocation_mode=InvocationMode.SUBPROCESS)`` to have the same behaviour Cosmos had in previous versions. Additional information `here <https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls>`_ and `here <https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#how-to-run-dbt-ls-invocation-mode>`_. Features * Use ``dbtRunner`` in the DAG Processor when using ``LoadMode.DBT_LS`` if ``dbt-core`` is available by @tatiana in #1484. Additional information `here <https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls>`_. * Allow users to opt-out of ``dbtRunner`` during DAG parsing with ``InvocationMode.SUBPROCESS`` by @tatiana in #1495. Check out the `documentation <https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#how-to-run-dbt-ls-invocation-mode>`_. * Add structure to support multiple db for async operator execution by @pankajastro in #1483 * Support overriding the ``profile_config`` per dbt node or folder using config by @tatiana in #1492. More information `here <https://astronomer.github.io/astronomer-cosmos/profiles/#profile-customise-per-node>`_. * Create and run accurate SQL statements when using ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti, @tatiana and @pankajastro in #1474 * Add AWS ECS task run execution mode by @CarlosGitto and @aoelvp94 in #1507 * Add support for running ``DbtSourceOperator`` individually by @victormacaubas in #1510 * Add setup task for async executions by @pankajastro in #1518 * Add teardown task for async executions by @pankajastro in #1529 * Add ``ProjectConfig.install_dbt_deps`` & change operator ``install_deps=True`` as default by @tatiana in #1521 * Extend Virtualenv operator and mock dbt adapters for setup & teardown tasks in ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti, @tatiana and @pankajastro in #1544 Bug Fixes * Fix select complex intersection of three tag-based graph selectors by @tatiana in #1466 * Fix custom selector behaviour when the model name contains periods by @yakovlevvs and @60098727 in #1499 * Filter dbt and non-dbt kwargs correctly for async operator by @pankajastro in #1526 Enhancement * Fix OpenLineage deprecation warning by @CorsettiS in #1449 * Move ``DbtRunner`` related functions into ``dbt/runner.py`` module by @tatiana in #1480 * Add ``on_warning_callback`` to ``DbtSourceKubernetesOperator`` and refactor previous operators by @LuigiCerone in #1501 * Gracefully error when users set incompatible ``RenderConfig.dbt_deps`` and ``operator_args`` ``install_deps`` by @tatiana in #1505 * Store compiled SQL as template field for ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti in #1534 Docs * Improve ``RenderConfig`` arguments documentation by @tatiana in #1514 * Improve callback documentation by @tatiana in #1516 * Improve partial parsing docs by @tatiana in #1520 * Fix typo in selecting & excluding docs by @pankajastro in #1523 * Document ``async_py_requirements`` added in ``ExecutionConfig`` for ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti in #1545 Others * Ignore dbt package tests when running Cosmos tests by @tatiana in #1502 * Refactor to consolidate async dbt adapter code by @pankajkoti in #1509 * Log elapsed time for sql file(s) upload/download by @pankajastro in #1536 * Remove the fallback operator for async task by @pankajastro in #1538 * GitHub Actions Dependabot: #1487 * Pre-commit updates: #1473, #1493, #1503, #1531

tatiana temporarily deployed to internal January 14, 2025 13:17 — with GitHub Actions Inactive

tatiana changed the title ~~WIP: Add support for intersection of "+tag:some_tag" in RenderConfig(select)~~ WIP: Investigate issue in the intersection of "+tag:some_tag" in RenderConfig(select) Jan 14, 2025

tatiana temporarily deployed to internal January 14, 2025 13:32 — with GitHub Actions Inactive

tatiana changed the title ~~WIP: Investigate issue in the intersection of "+tag:some_tag" in RenderConfig(select)~~ WIP: Investigate issue in the intersection of "+tag:some_tag" select statements Jan 14, 2025

tatiana requested review from pankajkoti and pankajastro January 15, 2025 09:50

tatiana temporarily deployed to internal January 20, 2025 12:58 — with GitHub Actions Inactive

tatiana added 5 commits January 20, 2025 12:58

Try to reproduce issue customer is facing with intersection of graph …

59dad89

…selectors

Example of test-case that reproduces problem faced by the customer

0fa488b

Add examples of using +tag:some_tag that work

aaf3010

Fix to customer issue

76d11c1

Release 1.9.0a1

dec5687

tatiana force-pushed the select-intersection-graphs branch from a4371f6 to dec5687 Compare January 20, 2025 13:01

tatiana temporarily deployed to internal January 20, 2025 13:01 — with GitHub Actions Inactive

Improve docs and add test

39166a8

tatiana temporarily deployed to internal January 20, 2025 15:43 — with GitHub Actions Inactive

tatiana changed the title ~~WIP: Investigate issue in the intersection of "+tag:some_tag" select statements~~ Investigate issue in the intersection of "+tag:some_tag" select statements Jan 20, 2025

tatiana marked this pull request as ready for review January 20, 2025 15:46

tatiana changed the title ~~Investigate issue in the intersection of "+tag:some_tag" select statements~~ Fix custom "+tag:some_tag" selector issue, related to test tag inheritance Jan 20, 2025

tatiana changed the title ~~Fix custom "+tag:some_tag" selector issue, related to test tag inheritance~~ Fix custom "+tag:some_tag" selector issue related to tests tag inheritance Jan 20, 2025

tatiana added this to the Cosmos 1.9.0 milestone Jan 20, 2025

pankajkoti approved these changes Jan 21, 2025

View reviewed changes

cosmos/dbt/selector.py Outdated Show resolved Hide resolved

tatiana commented Jan 21, 2025

View reviewed changes

cosmos/dbt/selector.py Outdated Show resolved Hide resolved

Update cosmos/dbt/selector.py

89bdf5b

tatiana temporarily deployed to internal January 21, 2025 10:54 — with GitHub Actions Inactive

Merge branch 'main' into select-intersection-graphs

c8cbfe9

tatiana temporarily deployed to internal January 21, 2025 10:55 — with GitHub Actions Inactive

tatiana merged commit 74c7c7d into main Jan 21, 2025
66 checks passed

tatiana deleted the select-intersection-graphs branch January 21, 2025 11:02

tatiana mentioned this pull request Jan 21, 2025

Review _should_include_node implementation, that changes test nodes tags #1479

Open

pankajkoti mentioned this pull request Feb 14, 2025

Release 1.9.0 #1539

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix custom "+tag:some_tag" selector issue related to tests tag inheritance #1466

Fix custom "+tag:some_tag" selector issue related to tests tag inheritance #1466

tatiana commented Jan 14, 2025 •

edited

Loading

netlify bot commented Jan 14, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading

pankajkoti left a comment

Fix custom "+tag:some_tag" selector issue related to tests tag inheritance #1466

Fix custom "+tag:some_tag" selector issue related to tests tag inheritance #1466

Conversation

tatiana commented Jan 14, 2025 • edited Loading

netlify bot commented Jan 14, 2025 • edited Loading

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

cloudflare-workers-and-pages bot commented Jan 14, 2025 • edited Loading

Deploying astronomer-cosmos with Cloudflare Pages

codecov bot commented Jan 14, 2025 • edited Loading

Codecov Report

pankajkoti left a comment

Choose a reason for hiding this comment

tatiana commented Jan 14, 2025 •

edited

Loading

netlify bot commented Jan 14, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading