Skip to content

Conversation

@tirkarthi
Copy link
Contributor

Fixes below warning in airflow tasks test command along with fixing test cases. Similar to PR #20781 and #20579

/home/karthikeyan/stuff/python/airflow/airflow/cli/commands/task_command.py:129 RemovedInAirflow3Warning: Calling `DAG.create_dagrun()` without an explicit data interval is deprecated

cc: @uranusjr

Copy link
Member

@uranusjr uranusjr Oct 18, 2022

Choose a reason for hiding this comment

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

This would not always work since infer_automated_data_interval is only designed to provide backward compatibility for DAG runs that are created before the data interval concept was introduced, and a DAG using a custom timetable would not work correctly with this method. I think infer_manual_data_interval is probably a better choice; it is not technically correct (the call below created a scheduled run), but the exact value of the data interval does not actually matter much here anyway since the run is only supposed to be a stub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, since run_type is not passed here this just calls the same part as underlying code to have same behavior. It's infer_manual_data_interval in tests since they pass run_type as manual. The dagrun is also deleted in the end so I guess it's ok.

airflow/airflow/models/dag.py

Lines 2582 to 2585 in ea55626

if run_type == DagRunType.MANUAL:
data_interval = self.timetable.infer_manual_data_interval(run_after=logical_date)
else:
data_interval = self.infer_automated_data_interval(logical_date)

Copy link
Member

Choose a reason for hiding this comment

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

I took the liberty to change this myself.

@potiuk
Copy link
Member

potiuk commented Oct 24, 2022

Conflicts needs resolving

@tirkarthi tirkarthi force-pushed the fix-task-test-warning branch from a42d747 to a8c9b31 Compare November 20, 2022 16:32
@tirkarthi
Copy link
Contributor Author

@potiuk Rebased with latest main branch and resolved conflicts.

@potiuk potiuk force-pushed the fix-task-test-warning branch from a8c9b31 to 89f6ef1 Compare November 25, 2022 22:04
@potiuk
Copy link
Member

potiuk commented Dec 3, 2022

Well. Not really all fixed.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 18, 2023
@tirkarthi tirkarthi force-pushed the fix-task-test-warning branch from 89f6ef1 to 001f18d Compare January 22, 2023 06:55
@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 23, 2023
@potiuk potiuk force-pushed the fix-task-test-warning branch from 001f18d to 4e29197 Compare February 18, 2023 13:10
@potiuk potiuk merged commit 70680de into apache:main Mar 16, 2023
@pierrejeambrun pierrejeambrun added this to the Airflow 2.5.3 milestone Mar 22, 2023
@pierrejeambrun pierrejeambrun added the type:misc/internal Changelog: Misc changes that should appear in change log label Mar 22, 2023
pierrejeambrun pushed a commit that referenced this pull request Mar 23, 2023
…nterval (#27106)

* Fix warning in airflow tasks test command regarding absence of data_interval.

* Use manual data interval in task commands

* Update tests/cli/commands/test_task_command.py

---------

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
(cherry picked from commit 70680de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants