Remove schedule_interval parameter
#503
Conversation
This change aligns with Airflow 2.10+ API changes where schedule_interval is deprecated in favor of the schedule parameter. The migration is handled automatically to ensure existing DAG configurations continue to work without modification.
This commit updates all test fixture files to use the new schedule parameter instead of the deprecated schedule_interval. All test configurations now reflect the current Airflow API standards. Files changed: - tests/fixtures/dag_factory.yml - tests/fixtures/dag_factory_task_group.yml - tests/fixtures/dag_factory_http_operator_task.yml - tests/fixtures/dag_factory_simple_http_operator_task.yml - tests/fixtures/dag_factory_kubernetes_pod_operator.yml - tests/fixtures/dag_factory_kubernetes_pod_operator_lt_2_7.yml - tests/fixtures/dag_factory_variables_as_arguments.yml - tests/fixtures/dag_factory_no_or_none_string_schedule.yml - tests/fixtures/dag_md_docs.yml - tests/fixtures/invalid_dag_factory.yml - tests/fixtures/invalid_yaml.yml
- Rename test_schedule_interval() to test_schedule() - Simplify test logic to use schedule attribute consistently - Update get_schedule_key() to always return "schedule" - Remove version-specific schedule_interval checks in tests - Update test configuration strings to use schedule parameter This commit updates the test suite to work with the new schedule parameter and removes legacy schedule_interval logic. Tests now consistently use the schedule attribute regardless of Airflow version, simplifying the test codebase and ensuring compatibility with future Airflow versions.
There was a problem hiding this comment.
Thank you, @viiccwen, for the contribution. Few tests are failing; you should be able to run them locally with hatch https://astronomer.github.io/dag-factory/latest/contributing/howto/#testing-application-with-hatch
| def get_schedule_key(): | ||
| airflow_version = version.parse(AIRFLOW_VERSION) | ||
| if airflow_version < version.parse("3.0.0"): | ||
| return "schedule_interval" | ||
| else: | ||
| return "schedule" | ||
| return "schedule" |
There was a problem hiding this comment.
I think now we can delete this function, no?
|
|
||
| if has_schedule_attr and not has_schedule_interval_attr and is_airflow_version_at_least_2_4: | ||
| if has_schedule_attr and is_airflow_version_at_least_2_4: |
There was a problem hiding this comment.
The minimum version of Airflow is 2.4 in DAG-Factory now so we can remove check is_airflow_version_at_least_2_4
There was a problem hiding this comment.
Thanks! I already see the minimum version.
| if utils.check_dict_key(dag_params, "schedule_interval"): | ||
| if dag_params["schedule_interval"] == "None": | ||
| dag_params["schedule"] = None | ||
| else: | ||
| dag_params["schedule"] = dag_params["schedule_interval"] | ||
| del dag_params["schedule_interval"] |
There was a problem hiding this comment.
I'm not sure if we should silently convert schedule_interval to schedule. Maybe in case of schedule_interval supplied, we should error out, or at least we should log this conversion.
There was a problem hiding this comment.
Since we're dropping support for airflow < 2.4 (including in our testing environment), maybe we should removed all schedule_interval handling and replaced it with a hard error.
I'll fix the tests first, and waiting for the reply : D
|
@pankajastro Thanks for replying and reviewing, I'll handle this ASAP! |
This PR refactors the usage of the deprecated
schedule_intervalparameter and replaces it with thescheduleparameter.This is my first pull request in this project!
If there’s anything I can improve or adjust, I’d really appreciate your feedback.
resolves #454