Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: ensure DbtGraph.update_node_dependency is called for all load methods #803

Merged

Conversation

jbandoro
Copy link
Collaborator

Description

This resolves #798 where when using LoadMode.DBT_LS_FILE, the DbtGraph.update_node_dependency was not called resulting in filtered nodes not having DbtNode.has_test set as expected.

Related Issue(s)

closes #798

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 January 17, 2024 20:05
@jbandoro jbandoro requested a review from a team January 17, 2024 20:05
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jan 17, 2024
Copy link

netlify bot commented Jan 17, 2024

Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name Link
🔨 Latest commit feda269
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/65a833099d201f000808be23

@dosubot dosubot bot added area:dependencies Related to dependencies, like Python packages, library versions, etc dbt:test Primarily related to dbt test command or functionality execution:local Related to Local execution environment parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing labels Jan 17, 2024
@jbandoro jbandoro added this to the 1.3.2 milestone Jan 17, 2024
Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8691c38) 94.62% compared to head (feda269) 94.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #803      +/-   ##
==========================================
- Coverage   94.62%   94.61%   -0.02%     
==========================================
  Files          55       55              
  Lines        2419     2413       -6     
==========================================
- Hits         2289     2283       -6     
  Misses        130      130              

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

Copy link
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.

This looks great, thanks for the fix, @jbandoro !

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 18, 2024
@tatiana tatiana merged commit b313544 into astronomer:main Jan 18, 2024
42 checks passed
tatiana pushed a commit that referenced this pull request Jan 26, 2024
…methods (#803)

This resolves #798 where when using `LoadMode.DBT_LS_FILE`, the
`DbtGraph.update_node_dependency` was not called resulting in filtered
nodes not having `DbtNode.has_test` set as expected.

Closes: #798
(cherry picked from commit b313544)
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL (#816)

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL (#816)

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL by @tatiana in #816

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
@tatiana tatiana mentioned this pull request Jan 26, 2024
tatiana added a commit that referenced this pull request Jan 26, 2024
**Bug fixes**

* Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load
methods by @jbandoro in #803
* Fix: ensure operator ``execute`` method is consistent across all
execution base subclasses by @jbandoro in #805
* Fix custom selector when ``test`` node has no ``depends_on`` values by
@tatiana in #814
* Fix forwarding selectors to test task when using
``TestBehavior.AFTER_ALL`` by @tatiana in #816

**Others**

* Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by
@jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and
@tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in
#812
* pre-commit updates in #799, #807
tatiana added a commit that referenced this pull request Jan 26, 2024
Bug fixes

* Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803
* Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805
* Fix custom selector when test node has no depends_on values by @tatiana in #814
* Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL by @tatiana in #816

Others

* Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in #812
* pre-commit updates in #799, #807
@tatiana tatiana mentioned this pull request Jan 27, 2024
tatiana added a commit that referenced this pull request Jan 27, 2024
**Bug fixes**

* Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load
methods by @jbandoro in #803
* Fix: ensure operator ``execute`` method is consistent across all
execution base subclasses by @jbandoro in #805
* Fix custom selector when ``test`` node has no ``depends_on`` values by
@tatiana in #814
* Fix forwarding selectors to test task when using
``TestBehavior.AFTER_ALL`` by @tatiana in #816

**Others**

* Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by
@jakob-hvitnov-telia in #797
* Add more logs to troubleshoot custom selector by @tatiana in #809
* Fix OpenLineage integration documentation by @tatiana in #810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and
@tatiana in #806
* Use Airflow constraint file for test environment setup by @jbandoro in
#812
* pre-commit updates in #799, #807

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com>
Co-authored-by: Jakob Aron Hvitnov <141235900+jakob-hvitnov-telia@users.noreply.github.com>
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
…methods (astronomer#803)

This resolves astronomer#798 where when using `LoadMode.DBT_LS_FILE`, the
`DbtGraph.update_node_dependency` was not called resulting in filtered
nodes not having `DbtNode.has_test` set as expected.

Closes: astronomer#798
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
**Bug fixes**

* Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load
methods by @jbandoro in astronomer#803
* Fix: ensure operator ``execute`` method is consistent across all
execution base subclasses by @jbandoro in astronomer#805
* Fix custom selector when ``test`` node has no ``depends_on`` values by
@tatiana in astronomer#814
* Fix forwarding selectors to test task when using
``TestBehavior.AFTER_ALL`` by @tatiana in astronomer#816

**Others**

* Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by
@jakob-hvitnov-telia in astronomer#797
* Add more logs to troubleshoot custom selector by @tatiana in astronomer#809
* Fix OpenLineage integration documentation by @tatiana in astronomer#810
* Fix test dependencies after Airflow 2.8 release by @jbandoro and
@tatiana in astronomer#806
* Use Airflow constraint file for test environment setup by @jbandoro in
astronomer#812
* pre-commit updates in astronomer#799, astronomer#807

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com>
Co-authored-by: Jakob Aron Hvitnov <141235900+jakob-hvitnov-telia@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dependencies Related to dependencies, like Python packages, library versions, etc dbt:test Primarily related to dbt test command or functionality execution:local Related to Local execution environment lgtm This PR has been approved by a maintainer parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LoadMode.DBT_LS_FILE does not render test nodes
2 participants