Skip to content

Add support for dbt selector arg for DAG parsing#755

Merged
tatiana merged 9 commits into
astronomer:mainfrom
jbandoro:718-add-support-for-dbt-selector-arg
Dec 13, 2023
Merged

Add support for dbt selector arg for DAG parsing#755
tatiana merged 9 commits into
astronomer:mainfrom
jbandoro:718-add-support-for-dbt-selector-arg

Conversation

@jbandoro
Copy link
Copy Markdown
Collaborator

@jbandoro jbandoro commented Dec 8, 2023

Description

This PR adds support for a new selector arg in RenderConfig so users can reference dbt yaml selectors when rendering a project using dbt ls by adding the --selector arg to the cli command. This was pretty straightforward for dbt ls, though in order to support custom and manifest filtering we would need to add logic for translating the yaml selector definitions in follow up PR(s).

Added a coverage test and integration test here, and a small update in tests/dbt/test_graph.py to use a fixture for a ProfileConfig that is repeated 9 times.

An open question I have is whether we want to log a warning in RenderConfig.__post_init__ if a user uses both select/exclude with selector. dbt docs here state that when using --selector with --exclude and/or --select the select/exclude flags are ignored (no error or warnings are logged by dbt if both are used).

Related Issue(s)

Closes #718

Breaking Change?

None

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

@jbandoro jbandoro requested a review from a team as a code owner December 8, 2023 21:19
@jbandoro jbandoro requested a review from a team December 8, 2023 21:19
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Dec 8, 2023
@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 8, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit a0bd087
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/6579d2ba5e4196000886c341

@dosubot dosubot Bot added area:config Related to configuration, like YAML files, environment variables, or executer configuration area:docs Relating to documentation, changes, fixes, improvement area:selector Related to selector, like DAG selector, DBT selector, etc area:testing Related to testing, like unit tests, integration tests, etc dbt:list Primarily related to dbt list command or functionality parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing labels Dec 8, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4369987) 93.18% compared to head (a0bd087) 93.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #755      +/-   ##
==========================================
+ Coverage   93.18%   93.20%   +0.01%     
==========================================
  Files          55       55              
  Lines        2451     2458       +7     
==========================================
+ Hits         2284     2291       +7     
  Misses        167      167              

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

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.

Hi @jbandoro, as usual, great work!

An open question is whether we want to log a warning in RenderConfig.post_init if a user uses both select/exclude with selector. dbt docs here state that when using --selector with --exclude and --select, the select/exclude flags are ignored (no error or warnings are logged by dbt if both are used).

I like the way you developed the feature. While checking for this in Cosmos would make users aware of the dbt limitation, if dbt decides to support both types of selectors simultaneously - we would have to change Cosmos to keep this.

The one check I think we need to add is to explicitly raise an exception if users try to use selector with a load mode that does not support it.

Do you think we should log a follow-up ticket to support this in the other load modes?

Comment thread docs/configuration/selecting-excluding.rst Outdated
Comment thread docs/configuration/selecting-excluding.rst
Comment thread tests/dbt/test_graph.py
Comment thread cosmos/dbt/graph.py
@jbandoro
Copy link
Copy Markdown
Collaborator Author

The one check I think we need to add is to explicitly raise an exception if users try to use selector with a load mode that does not support it.

Good point, I think that should be done in DbtGraph incase RenderConfig.load_method = LoadMode.AUTOMATIC. Will update.

Do you think we should log a follow-up ticket to support this in the other load modes?

Yes definitely, I can follow-up and create one after #718 is closed.

Co-authored-by: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>
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.

Looks great, thanks, @jbandoro for adding this feature. Once the checks pass, we can merge this PR.

@tatiana tatiana added the lgtm This PR has been approved by a maintainer label Dec 13, 2023
@tatiana tatiana merged commit 06e5574 into astronomer:main Dec 13, 2023
@jbandoro
Copy link
Copy Markdown
Collaborator Author

@tatiana added a follow-up issue for support manifest and custom load modes with the selector arg: #767

@tatiana tatiana mentioned this pull request Jan 4, 2024
tatiana added a commit that referenced this pull request Jan 4, 2024
**Features**

* Add new parsing method ``LoadMode.DBT_LS_FILE`` by @woogakoki in #733
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls-file)).
* Add support to select using (some) graph operators when using
``LoadMode.CUSTOM`` and ``LoadMode.DBT_MANIFEST`` by @tatiana in #728
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/selecting-excluding.html#using-select-and-exclude))
* Add support for dbt ``selector`` arg for DAG parsing by @jbandoro in
#755,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#render-config)).
* Add ``ProfileMapping`` for Vertica by @perttus in #540, #688 and #741,
as
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/VerticaUserPassword.html)).
* Add ``ProfileMapping`` for Snowflake encrypted private key path by
@ivanstillfront in #608, as ([documentation](
https://astronomer.github.io/astronomer-cosmos/profiles/SnowflakeEncryptedPrivateKeyFilePem.html)).
* Add support for Snowflake encrypted private key environment variable
by @DanMawdsleyBA in #649
* Add ``DbtDocsGCSOperator`` for uploading dbt docs to GCS by @jbandoro
in #616,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/generating-docs.html#upload-to-gcs)).
* Add cosmos/propagate_logs Airflow config support for disabling log
propagation by @agreenburg in #648,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/logging.html)).
* Add operator_args ``full_refresh`` as a templated field by @joppevos
in #623
* Expose environment variables and dbt variables in ``ProjectConfig`` by
@jbandoro in #735
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html#project-config-example)).
* Support disabling event tracking when using Cosmos profile mapping by
@jbandoro in #768,
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/index.html#disabling-dbt-event-tracking)).

**Enhancements**

* Make Pydantic an optional dependency by @pixie79 in #736
* Create a symbolic link to ``dbt_packages`` when ``dbt_deps`` is False
when using ``LoadMode.DBT_LS`` by @DanMawdsleyBA in #730
* Add ``aws_session_token`` for Athena mapping by @benjamin-awd in #663
* Retrieve temporary credentials from ``conn_id`` for Athena by @octiva
in #758
* Extend ``DbtDocsLocalOperator`` with static flag by @joppevos  in #759

**Bug fixes**

* Remove Pydantic upper version restriction so Cosmos can be used with
Airflow 2.8 by @jlaneve in #772

**Others**

* Replace flake8 for Ruff by @joppevos in #743
* Reduce code complexity to 8 by @joppevos in #738
* Speed up integration tests by @jbandoro in #732
* Fix README quickstart link in by @RNHTTR in #776
* Add package location to work with hatchling 1.19.0 by @jbandoro in
#761
* Fix type check error in ``DbtKubernetesBaseOperator.build_env_args``
by @jbandoro in #766
* Improve ``DBT_MANIFEST`` documentation by @dwreeves in #757
* Update conflict matrix between Airflow and dbt versions by @tatiana in
#731 and #779
* pre-commit updates in #775, #770, #762
ykuc pushed a commit to ykuc/astronomer-cosmos that referenced this pull request Jan 11, 2024
**Features**

* Add new parsing method ``LoadMode.DBT_LS_FILE`` by @woogakoki in astronomer#733
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls-file)).
* Add support to select using (some) graph operators when using
``LoadMode.CUSTOM`` and ``LoadMode.DBT_MANIFEST`` by @tatiana in astronomer#728
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/selecting-excluding.html#using-select-and-exclude))
* Add support for dbt ``selector`` arg for DAG parsing by @jbandoro in
astronomer#755,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#render-config)).
* Add ``ProfileMapping`` for Vertica by @perttus in astronomer#540, astronomer#688 and astronomer#741,
as
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/VerticaUserPassword.html)).
* Add ``ProfileMapping`` for Snowflake encrypted private key path by
@ivanstillfront in astronomer#608, as ([documentation](
https://astronomer.github.io/astronomer-cosmos/profiles/SnowflakeEncryptedPrivateKeyFilePem.html)).
* Add support for Snowflake encrypted private key environment variable
by @DanMawdsleyBA in astronomer#649
* Add ``DbtDocsGCSOperator`` for uploading dbt docs to GCS by @jbandoro
in astronomer#616,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/generating-docs.html#upload-to-gcs)).
* Add cosmos/propagate_logs Airflow config support for disabling log
propagation by @agreenburg in astronomer#648,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/logging.html)).
* Add operator_args ``full_refresh`` as a templated field by @joppevos
in astronomer#623
* Expose environment variables and dbt variables in ``ProjectConfig`` by
@jbandoro in astronomer#735
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html#project-config-example)).
* Support disabling event tracking when using Cosmos profile mapping by
@jbandoro in astronomer#768,
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/index.html#disabling-dbt-event-tracking)).

**Enhancements**

* Make Pydantic an optional dependency by @pixie79 in astronomer#736
* Create a symbolic link to ``dbt_packages`` when ``dbt_deps`` is False
when using ``LoadMode.DBT_LS`` by @DanMawdsleyBA in astronomer#730
* Add ``aws_session_token`` for Athena mapping by @benjamin-awd in astronomer#663
* Retrieve temporary credentials from ``conn_id`` for Athena by @octiva
in astronomer#758
* Extend ``DbtDocsLocalOperator`` with static flag by @joppevos  in astronomer#759

**Bug fixes**

* Remove Pydantic upper version restriction so Cosmos can be used with
Airflow 2.8 by @jlaneve in astronomer#772

**Others**

* Replace flake8 for Ruff by @joppevos in astronomer#743
* Reduce code complexity to 8 by @joppevos in astronomer#738
* Speed up integration tests by @jbandoro in astronomer#732
* Fix README quickstart link in by @RNHTTR in astronomer#776
* Add package location to work with hatchling 1.19.0 by @jbandoro in
astronomer#761
* Fix type check error in ``DbtKubernetesBaseOperator.build_env_args``
by @jbandoro in astronomer#766
* Improve ``DBT_MANIFEST`` documentation by @dwreeves in astronomer#757
* Update conflict matrix between Airflow and dbt versions by @tatiana in
astronomer#731 and astronomer#779
* pre-commit updates in astronomer#775, astronomer#770, astronomer#762
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
=This PR adds support for a new `selector` arg in `RenderConfig` so users
can reference[ dbt yaml
selectors](https://docs.getdbt.com/reference/node-selection/yaml-selectors)
when rendering a project using dbt ls by adding the `--selector` arg to
the cli command. This was pretty straightforward for dbt ls, though in
order to support custom and manifest filtering we would need to add
logic for translating the yaml selector definitions in follow up PR(s).

Added a coverage test and integration test here, and a small update in
`tests/dbt/test_graph.py` to use a fixture for a ProfileConfig that is
repeated 9 times.

An open question I have is whether we want to log a warning in
`RenderConfig.__post_init__` if a user uses both `select`/`exclude` with
`selector`. dbt docs
[here](https://docs.getdbt.com/reference/node-selection/syntax#examples)
state that when using `--selector` with `--exclude` and/or `--select`
the select/exclude flags are ignored (no error or warnings are logged by
dbt if both are used).

Closes astronomer#718
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
**Features**

* Add new parsing method ``LoadMode.DBT_LS_FILE`` by @woogakoki in astronomer#733
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls-file)).
* Add support to select using (some) graph operators when using
``LoadMode.CUSTOM`` and ``LoadMode.DBT_MANIFEST`` by @tatiana in astronomer#728
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/selecting-excluding.html#using-select-and-exclude))
* Add support for dbt ``selector`` arg for DAG parsing by @jbandoro in
astronomer#755,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#render-config)).
* Add ``ProfileMapping`` for Vertica by @perttus in astronomer#540, astronomer#688 and astronomer#741,
as
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/VerticaUserPassword.html)).
* Add ``ProfileMapping`` for Snowflake encrypted private key path by
@ivanstillfront in astronomer#608, as ([documentation](
https://astronomer.github.io/astronomer-cosmos/profiles/SnowflakeEncryptedPrivateKeyFilePem.html)).
* Add support for Snowflake encrypted private key environment variable
by @DanMawdsleyBA in astronomer#649
* Add ``DbtDocsGCSOperator`` for uploading dbt docs to GCS by @jbandoro
in astronomer#616,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/generating-docs.html#upload-to-gcs)).
* Add cosmos/propagate_logs Airflow config support for disabling log
propagation by @agreenburg in astronomer#648,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/logging.html)).
* Add operator_args ``full_refresh`` as a templated field by @joppevos
in astronomer#623
* Expose environment variables and dbt variables in ``ProjectConfig`` by
@jbandoro in astronomer#735
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html#project-config-example)).
* Support disabling event tracking when using Cosmos profile mapping by
@jbandoro in astronomer#768,
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/index.html#disabling-dbt-event-tracking)).

**Enhancements**

* Make Pydantic an optional dependency by @pixie79 in astronomer#736
* Create a symbolic link to ``dbt_packages`` when ``dbt_deps`` is False
when using ``LoadMode.DBT_LS`` by @DanMawdsleyBA in astronomer#730
* Add ``aws_session_token`` for Athena mapping by @benjamin-awd in astronomer#663
* Retrieve temporary credentials from ``conn_id`` for Athena by @octiva
in astronomer#758
* Extend ``DbtDocsLocalOperator`` with static flag by @joppevos  in astronomer#759

**Bug fixes**

* Remove Pydantic upper version restriction so Cosmos can be used with
Airflow 2.8 by @jlaneve in astronomer#772

**Others**

* Replace flake8 for Ruff by @joppevos in astronomer#743
* Reduce code complexity to 8 by @joppevos in astronomer#738
* Speed up integration tests by @jbandoro in astronomer#732
* Fix README quickstart link in by @RNHTTR in astronomer#776
* Add package location to work with hatchling 1.19.0 by @jbandoro in
astronomer#761
* Fix type check error in ``DbtKubernetesBaseOperator.build_env_args``
by @jbandoro in astronomer#766
* Improve ``DBT_MANIFEST`` documentation by @dwreeves in astronomer#757
* Update conflict matrix between Airflow and dbt versions by @tatiana in
astronomer#731 and astronomer#779
* pre-commit updates in astronomer#775, astronomer#770, astronomer#762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:config Related to configuration, like YAML files, environment variables, or executer configuration area:docs Relating to documentation, changes, fixes, improvement area:selector Related to selector, like DAG selector, DBT selector, etc area:testing Related to testing, like unit tests, integration tests, etc dbt:list Primarily related to dbt list command or functionality lgtm This PR has been approved by a maintainer parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support to dbt selector argument, in addition to select and exclude

2 participants