Skip to content

Added callback_file & callback_name to default_args DAG level and tests#218

Merged
pankajkoti merged 2 commits into
astronomer:mainfrom
subbota19:main
Oct 16, 2024
Merged

Added callback_file & callback_name to default_args DAG level and tests#218
pankajkoti merged 2 commits into
astronomer:mainfrom
subbota19:main

Conversation

@subbota19
Copy link
Copy Markdown
Contributor

@subbota19 subbota19 commented Sep 14, 2024

Hi astro team!

This PR brings the following enhancements:

  1. Unittest for DAG-Level Callback using parameters:

on_success_callback_name & on_success_callback_file
on_failure_callback_name & on_failure_callback_file

Astro instruction on how to apply these parameters

  1. While I find the existing callback feature useful which allows specifying callback code at any location, this PR takes it a step further by enabling callbacks to be specified within the DAG's default_args. With this enhancement, the callbacks will automatically propagate to the task level as well. This update aligns with how default_args are passed in Airflow. Additionally, I’ve included unit tests.

@subbota19 subbota19 mentioned this pull request Sep 14, 2024
Closed
tatiana
tatiana reviewed Sep 20, 2024
View reviewed changes
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.

This is super helpful, @subbota19 , thank you very much for working on this!
I'm happy for us to merge this change once we confirm the tests are passing.

@subbota19
Copy link
Copy Markdown
Contributor Author

subbota19 commented Sep 20, 2024

Hi @tatiana !
I've added another commit to address the failed fmt-check step. The code is now formatted correctly, could we please rerun the workflow one more time?

@subbota19 subbota19 requested a review from tatiana September 29, 2024 08:47
@tatiana tatiana added this to the DAG Factory 0.20.0 milestone Oct 7, 2024
@pankajkoti pankajkoti self-requested a review October 10, 2024 14:00
pankajkoti
pankajkoti approved these changes Oct 10, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the contribution @subbota19

@tatiana can we please check why the project settings is not asking to rebase/update the PR from main before merging?

@pankajastro
Copy link
Copy Markdown
Contributor

pankajastro commented Oct 14, 2024

For some reason, I still can't see the update button. I'm looking for something like below (Cosmos).

Screenshot 2024-10-15 at 1 08 57 AM

@pankajkoti pankajkoti added bug Something isn't working and removed bug Something isn't working labels Oct 16, 2024
@pankajkoti pankajkoti closed this Oct 16, 2024
@pankajkoti pankajkoti reopened this Oct 16, 2024
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.67%. Comparing base (e474a3a) to head (b672b65).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #218      +/-   ##
==========================================
+ Coverage   87.24%   87.67%   +0.42%     
==========================================
  Files           6        6              
  Lines         580      584       +4     
==========================================
+ Hits          506      512       +6     
+ Misses         74       72       -2

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

@pankajkoti
Copy link
Copy Markdown
Contributor

pankajkoti commented Oct 16, 2024

I was able to test the support for included callbacks in default_args with this change. The CI is happy except for Static-Check failures; since I cannot push to this PR, I will open a follow-up PR to fix the failure. Am going ahead with merging this PR.

@pankajkoti pankajkoti merged commit 93e32e6 into astronomer:main Oct 16, 2024
pankajkoti added a commit that referenced this pull request Oct 16, 2024
@pankajkoti
Fix static check failures in PR #218
8b5b0e8
@pankajkoti pankajkoti mentioned this pull request Oct 16, 2024
Merged
pankajkoti added a commit that referenced this pull request Oct 16, 2024
@pankajkoti
Fix static check failures in PR #218 (#251)
6f31846 
Fix the failed Static Check job here that were observed in #218.
Since the PR was created from a fork, I could not push changes to it
before merging.

related: #206
@pankajkoti pankajkoti mentioned this pull request Oct 16, 2024
Closed
5 tasks
@tatiana tatiana mentioned this pull request Oct 17, 2024
Merged
tatiana added a commit that referenced this pull request Oct 22, 2024
@tatiana
Release 0.20.0 (#254)
d246e8c 
### Added
- Support using envvar in config YAML by @tatiana in #236
- **Callback improvements**
- Support installed code via python callable string by @john-drews in
#221
- Add `callback_file` & `callback_name` to `default_args` DAG level by
@subbota19 in #218
- Cast callbacks to functions when set with `default_args` on TaskGroups
by @baraldo and @pankajastro in #235

- **Telemetry**
- For more information, please, read the [Privacy
Notice](https://github.com/astronomer/dag-factory/blob/main/PRIVACY_NOTICE.md#collection-of-data).
  - Add scarf to readme for website analytics by @cmarteepants in #219
- Support telemetry during DAG parsing emitting data to Scarf by
@tatiana in #250.

### Fixed
- Build DAGs when tehre is an invalid YAML in the DAGs folder by @quydx
and @tatiana in #184

### Others
- Development tools
  - Fix make docker-run by @pankajkoti in #249
  - Add vim dot files to .gitignore by @tatiana in #228
  - Use Hatchling to modern package building by @kaxil in #208
- CI
  - Fix static check failures in PR #218 by @pankajkoti in #251
  - Fix pre-commit checks by @tatiana in #247
  - Remove tox and corresponding build jobs in CI by @pankajkoti in #248
- Install Airflow with different versions in the CI by @pankajkoti in
#237
  - Run pre-commit hooks on all existing files by @pankajkoti in #245
  - Add Python 3.11 and 3.12 to CI test pipeline by @pankajkoti in #229
- Tests
  - Fix duplicate test name by @pankajastro in #234
  - Add static check by @pankajastro in #231
  - Fix running tests locally (outside the CI) by @tatiana in #227
  - Add the task_2 back to dataset example by @cmarteepants in #204
  - Remove unnecessary config line by @jlaneve in #202
- Documentation
  - Update the license from MIT to Apache 2.0 by @pankajastro in #191
- Add registration icon and links to Airflow references by @cmarteepants
in #190
  - Update quickstart and add feature examples by @cmarteepants #189

### Breaking changes
- Removed support for Python 3.7
- The license was changed from MIT to Apache 2.0

Closes: #217
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pankajkoti pankajkoti pankajkoti approved these changes

@tatiana tatiana Awaiting requested review from tatiana

Assignees

@pankajkoti pankajkoti

Labels

None yet

Projects

None yet

Milestone

DAG Factory 0.20.0

Development

Successfully merging this pull request may close these issues.

5 participants

@subbota19 @pankajastro @codecov-commenter @pankajkoti @tatiana