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 parsing test nodes when using the custom load method (LoadMethod.CUSTOM) #563

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

raffifu
Copy link
Contributor

@raffifu raffifu commented Sep 28, 2023

Description

This PR Added a functionality to parse Test on the custom parser. After implementing #543, there's an issue using load_via_custom_parser which doesn't load the test node. That's happen because the implementation in #543 is assume all node doesn't has test. After debugging, i found that the load_via_custom_parser not return any tests node.

Related Issue(s)

closes #561

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

@raffifu raffifu requested a review from a team as a code owner September 28, 2023 08:27
@raffifu raffifu requested a review from a team September 28, 2023 08:27
@netlify
Copy link

netlify bot commented Sep 28, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit e37329f
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/651538fdf5f572000837a4bb

@raffifu raffifu temporarily deployed to external September 28, 2023 08:27 — with GitHub Actions Inactive
@raffifu raffifu temporarily deployed to external September 28, 2023 10:12 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (4c8d6b0) 92.88% compared to head (e37329f) 92.83%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #563      +/-   ##
==========================================
- Coverage   92.88%   92.83%   -0.06%     
==========================================
  Files          49       49              
  Lines        1968     1982      +14     
==========================================
+ Hits         1828     1840      +12     
- Misses        140      142       +2     
Files Coverage Δ
cosmos/__init__.py 100.00% <100.00%> (ø)
cosmos/dbt/graph.py 98.78% <100.00%> (ø)
cosmos/operators/local.py 90.87% <100.00%> (+0.05%) ⬆️
cosmos/dbt/parser/project.py 90.04% <94.44%> (-0.43%) ⬇️

☔ 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.

Thanks for this impressive quick fix, @raffifu!

Is there any chance you could add a unit test to cover
cosmos/dbt/parser/project.py#L368?

We'll release this fix as part of the 1.2 release - we should have an alpha pre-release soon.

@tatiana tatiana added this to the 1.2.0 milestone Sep 28, 2023
@tatiana tatiana changed the title Parse test on custom parser Fix parsing test nodes when using the custom load method (LoadMethod.CUSTOM) Sep 28, 2023
@tatiana tatiana merged commit 0123c3e into astronomer:main Sep 28, 2023
40 of 41 checks passed
tatiana pushed a commit that referenced this pull request Sep 28, 2023
…CUSTOM) (#563)

This PR fixes parsing test nodes when using LoadMethod.CUSTOM, since it didn't return any test nodes.
This issue surfaced after #543.

Closes: #561
(cherry picked from commit 0123c3e)
tatiana added a commit that referenced this pull request Sep 28, 2023
Bug fixes

* Only create task group and test task only if the model has a test by @raffifu in #543
* Fix parsing test nodes when using the custom load method (LoadMethod.CUSTOM) by @raffifu in #563
* Fix ``DbtTestOperator`` when test does not have ``test_metadata`` by @javihernovoa and @tatiana in #565
* Support dbt 1.6 and apache-airflow-providers-cncf-kubernetes 7.3.0  by @tatiana in #564
@tatiana tatiana mentioned this pull request Sep 28, 2023
tatiana added a commit that referenced this pull request Sep 28, 2023
Bug fixes

* Only create task group and test task only if the model has a test by @raffifu in #543
* Fix parsing test nodes when using the custom load method (LoadMethod.CUSTOM) by @raffifu in #563
* Fix ``DbtTestOperator`` when test does not have ``test_metadata`` by @javihernovoa and @tatiana in #565
* Support dbt 1.6 and apache-airflow-providers-cncf-kubernetes 7.3.0  by @tatiana in #564
tatiana added a commit that referenced this pull request Sep 29, 2023
Bug fixes
    
* Only create task group and test task only if the model has a test by
@raffifu in #543
* Fix parsing test nodes when using the custom load method
(`LoadMethod.CUSTOM`) by @raffifu in #563
* Fix `DbtTestOperator` when test does not have ``test_metadata`` by
@javihernovoa and @tatiana in #565
* Support dbt 1.6 and `apache-airflow-providers-cncf-kubernetes` 7.3.0
by @tatiana in #564
@tatiana
Copy link
Collaborator

tatiana commented Oct 13, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix dbt project parsing/Airflow DAG rendering for load_method=LoadMode.CUSTOM
2 participants