Skip to content

Remediated default behavior, added documentation#378

Merged
pankajastro merged 3 commits into
astronomer:mainfrom
jroachgolf84:issue-295
Mar 28, 2025
Merged

Remediated default behavior, added documentation#378
pankajastro merged 3 commits into
astronomer:mainfrom
jroachgolf84:issue-295

Conversation

@ghost

@ghost ghost commented Mar 6, 2025

Copy link
Copy Markdown

#295 outlined a documentation change that would better outline the workflow for defining and using a "default" block when creating DAGs with DAG Factory. In its most basic use, a default block at the top of a YAML file would be used, followed by the DAG-definitions leveraging these default values.

During the documentation and testing process, it was discovered that the previously-expected behavior was not being implemented. This was due to this code (https://github.com/astronomer/dag-factory/blob/main/dagfactory/dagfactory.py#L136C1-L140C18), which completely overrode default_config, rather than just the default_args key-value pair.

To handle this, both a change to the logic above as well the appropriate unit-tests were written. This change allowed for the behavior in #295 to be completely implemented and documented.

@ghost ghost self-requested a review as a code owner March 6, 2025 17:06
@codecov-commenter

codecov-commenter commented Mar 6, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.04%. Comparing base (0f8aad3) to head (aaa5fe7).
⚠️ Report is 174 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #378   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files          11       11           
  Lines         890      890           
=======================================
  Hits          837      837           
  Misses         53       53           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ghost

ghost commented Mar 10, 2025

Copy link
Copy Markdown
Author

cc: @pankajastro - ready for review!

@tatiana tatiana left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @jroach-astronomer , thanks for these contributions!

My understanding is that there are two types of changes in this PR:

  1. Linting improvements
  2. Fix/change of behaviour of default config in the YAML file

How did you come up with these linting changes? Was it using the project's pre-commit? It would be great if we separated these from the (2)

@ghost

ghost commented Mar 10, 2025

Copy link
Copy Markdown
Author

Hi @jroach-astronomer , thanks for these contributions!

My understanding is that there are two types of changes in this PR:

  1. Linting improvements
  2. Fix/change of behaviour of default config in the YAML file

How did you come up with these linting changes? Was it using the project's pre-commit? It would be great if we separated these from the (2)

@tatiana, that's right, these changes were made using pre-commit. What's the best way to separate these changes out?

@tatiana tatiana added this to the DAG Factory 0.23.0 milestone Mar 25, 2025
Comment thread dagfactory/dagfactory.py Outdated
Comment thread dev/dags/customized/callables/python.py
Comment thread docs/configuration/defaults.md
@ghost

ghost commented Mar 28, 2025

Copy link
Copy Markdown
Author

@pankajastro , I've resolved our conversations and made the changes you've requested.

@pankajastro

Copy link
Copy Markdown
Contributor

Hi @jroach-astronomer, Look like there are some conflicts. Could you please resolve them?

@ghost

ghost commented Mar 28, 2025

Copy link
Copy Markdown
Author

Hi @jroach-astronomer, Look like there are some conflicts. Could you please resolve them?

Merge conflict resolved!

@pankajastro pankajastro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for improving it.

@pankajastro pankajastro merged commit ae99563 into astronomer:main Mar 28, 2025
@pankajastro pankajastro mentioned this pull request Mar 29, 2025
pankajastro added a commit that referenced this pull request Jul 14, 2025
## [0.23.0] - 2025-07-14

### Breaking Change

- Drop Airflow 2.2 Support by @pankajastro in
[#388](#388)
- Drop Python 3.8 support by @pankajastro in
[#435](#435)
- Drop Airflow 2.3 Support by @pankajastro in
[#456](#456)

### Airflow 3 Support

- Add tools for Dag-factory Airflow3 testing by @pankajastro in
[#395](#395)
- Fix schedule args for Airflow 3 by @pankajastro in
[#413](#413)
- Fix import path for BranchPythonOperator, PythonOperator and
PythonSensor by @pankajastro in
[#414](#414)
- Add scheduling docs for Airflow 3 by @pankajastro in
[#424](#424)
- Enable Airflow 3 tests in CI by @pankajastro in
[#436](#436)
- Add env AUTO_CONVERT_TO_AF3 in Dockerfile by @pankajastro in
[#455](#455)
- Validate DAG's on Airflow 3 by @pankajastro in
[#457](#457)
- Refactor schedule to use the Python object @pankajastro in
[#458](#458)
- Fix CI and import issues for Airflow 3 compatibility @pankajastro in
[#463](#463)

### Added

- Add support for defining inlets by @IvanSviridov in
[#380](#380)
- Add HttpOperator JSON serialization support with tests by @a-chumagin
in [#382](#382)
- Add support for custom Python object by @pankajastro in
[#444](#444)
- Support env var in default by @gyli in
[#452](#452)
- Pass default arguments via dictionary in .py file by @jroachgolf84 in
[#465](#465)

### Fixed

- Remediated ``default`` behavior, added documentation by @jroachgolf84
in [#378](#378)
- Upgrade `apache-airflow-providers-cncf-kubernetes` provider by
@pankajastro in
[#407](#407)
- Include error message trace in exception by @pankajastro in
[#408](#408)

### Docs

- Remove Unreleased heading section from the CHANGELOG.md by @pankajkoti
in [#365](#365)
- Add Documentation for Conditional Dataset Scheduling with dag-factory
by @ErickSeo in
[#367](#367)
- Add copy right in docs footer by @pankajastro in
[#371](#371)
- Updating docs for callbacks by @jroachgolf84 in
[#375](#375)
- Add stable/latest version in docs by @pankajastro in
[#391](#391)
- Migrate old content to new documentation structure by @pankajastro in
[#393](#393)
- Update Airflow supported version 2.3+ in docs by @pankajastro in
[#412](#412)
- Doc: Add step to fork repo in contributing guide by @pankajastro in
[#427](#427)
- Add setting CONFIG_ROOT_DIR in the contribution doc by @gyli in
[#432](#432)
- Add DAG example showcasing runtime params usage by @pankajastro in
[#449](#449)
- Add jinja2 template example by @pankajastro in
[#450](#450)

### Others

- Add Scraf Pixels for telemetry by @pankajastro in
[#373](#373)
- feat: bumped http provider versions to 2.0+ by @a-chumagin in
[#389](#389)
- Add --verbosity debug in astro-cli cmd by @pankajastro in
[#390](#390)
- Add missing Python file for dynamic task example by @pankajastro in
[#392](#392)
- Pin apache-airflow-providers-cncf-kubernetes<10.4.2 by @pankajastro in
[#400](#400)
- Add script to check version and tag by @pankajastro in
[#395](#394)
- Assert DagRunState in integration test by @pankajastro in
[#415](#415)
- Move to uv for package management by @jlaneve in
[#419](#419)
- Install uv in CI by @jlaneve in
[#421](#421)
- Bump astral-sh/setup-uv from 5 to 6 by @dependabot in
[#423](#423)
- Add Airflow 2.11 in test matrix by @pankajastro in
[#425](#425)
- fix example_dag_factory.yml typo causing catchup: false to not be
respected by @RNHTTR in
[#431](#431)
- Remove expandvars in utils.get_python_callable by @gyli in
[#440](#440)
- Delete unused img folder by @pankajastro in
[#446](#446)
- Clean print statement by @pankajastro
[#447](#447)
- Add hatch to uv dev dependencies by @gyli in
[#453](#453)
- Add Authorize Job in CI by @pankajastro in
[#460](#460)
- Remove PyPI token for releasing packages by @tatiana in
[#461](#461)
- Change CI on trigger event to pull_request from pull_request_target by
@pankajkoti in
[#464](#464)
- Update cicd.yaml: Use pull_request for authorize as we don't have
pull_request_target event configured by @pankajkoti in
[#466](#466)
- Add environment for pypi publish job by @pankajastro in
[#478](#478)
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.

4 participants