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

Cast callbacks to functions when set with default_args on task groups #235

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

pankajastro
Copy link
Contributor

@pankajastro pankajastro commented Oct 10, 2024

Hello!

When used in the task group's default_args, callbacks are passed as strings to the tasks, which ends up generating failures.

The idea is to perform the same parse used in make_task (it is necessary to check whether the default_args attribute is defined, since it is optional and defaults to None) and perhaps in the future modularize this conversion functionality in order to cover the widespread use of default_args.

Co-authored-by: Luiz Felipe de Mesquita Baraldo luiz.felipe.m.baraldo@hotmail.com

Note: The actual work is done PR #174

Additional work

  • Rebase with main
  • Fixed static checks

@pankajastro pankajastro self-assigned this Oct 10, 2024
@pankajastro pankajastro marked this pull request as ready for review October 10, 2024 17:11
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 addressing this, @pankajastro ! Should we merge #174 first and then immediately after this PR, to fix the static checks that are not passing? Please improve the description of the PR with what we're accomplishing in this PR.

@pankajkoti
Copy link
Contributor

It appears to me that we have not enabled the setting to update branches/PR to latest main before merging the PR. Can we please enable that?

>

Co-authored-by: Luiz Felipe de Mesquita Baraldo <luiz.felipe.m.baraldo@hotmail.com>
@pankajastro
Copy link
Contributor Author

Thanks for addressing this, @pankajastro ! Should we merge #174 first and then immediately after this PR, to fix the static checks that are not passing? Please improve the description of the PR with what we're accomplishing in this PR.

This is a replication of #17 only. I attempted to address the checks in the original PR, but it became too complicated to implement changes there

@pankajastro pankajastro merged commit 6948ad5 into main Oct 14, 2024
7 checks passed
@pankajastro pankajastro deleted the fix-174 branch October 14, 2024 18:16
@pankajastro pankajastro added this to the DAG Factory 0.20.0 milestone Oct 14, 2024
@tatiana tatiana mentioned this pull request Oct 17, 2024
tatiana added a commit that referenced this pull request Oct 22, 2024
### 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants