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

Remove prefix of run and test tasks if test_behavior = TestBehavior.AFTER_EACH #524

Merged
merged 5 commits into from
Sep 11, 2023

Conversation

binhnq94
Copy link
Contributor

@binhnq94 binhnq94 commented Sep 8, 2023

Description

If test_behavior = TestBehavior.AFTER_EACH, DbtNode.name in task_id is not necessary because the parent task group is named as DbtNode.name

Related Issue(s)

Slack thread

Breaking Change?

Remove prefix of run and test tasks if test_behavior = TestBehavior.AFTER_EACH.

Olds rendered tasks before this PR:
image

New rendered tasks:
image

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

@binhnq94 binhnq94 requested a review from a team as a code owner September 8, 2023 19:06
@binhnq94 binhnq94 requested a review from a team September 8, 2023 19:06
@netlify
Copy link

netlify bot commented Sep 8, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit 300691a
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/64ff24ffe523bd00085e2f1b

@pre-commit-ci pre-commit-ci bot temporarily deployed to external September 8, 2023 19:07 Inactive
@pre-commit-ci pre-commit-ci bot temporarily deployed to external September 11, 2023 09:28 Inactive
@tatiana tatiana temporarily deployed to external September 11, 2023 10:20 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (dd0dcb6) 91.78% compared to head (300691a) 91.80%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #524      +/-   ##
==========================================
+ Coverage   91.78%   91.80%   +0.01%     
==========================================
  Files          49       49              
  Lines        1911     1915       +4     
==========================================
+ Hits         1754     1758       +4     
  Misses        157      157              
Files Changed Coverage Δ
cosmos/airflow/graph.py 100.00% <100.00%> (ø)

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

@binhnq94 binhnq94 temporarily deployed to external September 11, 2023 11:14 — with GitHub Actions Inactive
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 pointing out the redundancy in the TaskGroup and the inner Task names - this looks cleaner!

I just checked how it looked in the bigger view of the DAG, and it makes more sense:

Screenshot 2023-09-11 at 15 14 27

Screenshot 2023-09-11 at 15 14 51

I also confirmed that things looked good in the Task Instance list view and the Airflow Grid and Task Tries tabs.

@binhnq94 binhnq94 temporarily deployed to external September 11, 2023 14:32 — with GitHub Actions Inactive
@tatiana tatiana merged commit 2e8f0a8 into astronomer:main Sep 11, 2023
40 checks passed
tatiana added a commit that referenced this pull request Sep 14, 2023
Bug fixes

* Fix attempt of emitting OpenLineage events if task execution fails by @tatiana in #526
* Fix Rust dependency for Windows users by @tatiana in #526
* Fix DbtRunOperationLocalOperator missing flags by @tatiana in #529
* Fix DbtRunLocalOperator to support the full refresh argument by @tatiana in #529
* Remove redundant prefix of task names when test_behavior = TestBehavior.AFTER_EACH by @binhnq94 in #524
* Fix rendering vars in `DbtModel` when using `LoadMode.CUSTOM` by @dojinkimm in #502

Others

* Docs: add `documentation comparing Airflow and dbt concepts <https://astronomer.github.io/astronomer-cosmos/getting_started/dbt-airflow-concepts.html>`_ by @tatiana in #523.
* Update PyPI project links by @tatiana in #528
* pre-commit updates
tatiana added a commit that referenced this pull request Sep 14, 2023
Bug fixes

* Fix attempt of emitting OpenLineage events if task execution fails by @tatiana in #526
* Fix Rust dependency for Windows users by @tatiana in #526
* Fix DbtRunOperationLocalOperator missing flags by @tatiana in #529
* Fix DbtRunLocalOperator to support the full refresh argument by @tatiana in #529
* Remove redundant prefix of task names when test_behavior = TestBehavior.AFTER_EACH by @binhnq94 in #524
* Fix rendering vars in `DbtModel` when using `LoadMode.CUSTOM` by @dojinkimm in #502

Others

* Docs: add `documentation comparing Airflow and dbt concepts <https://astronomer.github.io/astronomer-cosmos/getting_started/dbt-airflow-concepts.html>`_ by @tatiana in #523.
* Update PyPI project links by @tatiana in #528
* pre-commit updates
@tatiana tatiana mentioned this pull request Sep 14, 2023
harels pushed a commit that referenced this pull request Sep 14, 2023
Bug fixes

* Remove redundant prefix of task names when test_behavior =
TestBehavior.AFTER_EACH by @binhnq94 in #524
* Fix rendering vars in `DbtModel` when using `LoadMode.CUSTOM` by
@dojinkimm in #502
* Fix attempt of emitting OpenLineage events if task execution fails by
@tatiana in #526
* Fix Rust dependency for Windows users by @tatiana in #526
* Fix DbtRunOperationLocalOperator missing flags by @tatiana in #529
* Fix DbtRunLocalOperator to support the full refresh argument by
@tatiana in #529

Others

* Docs: add `documentation comparing Airflow and dbt concepts
<https://astronomer.github.io/astronomer-cosmos/getting_started/dbt-airflow-concepts.html>`_
by @tatiana in #523.
* Update PyPI project links by @tatiana in #528
* pre-commit updates
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.

2 participants