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

Remove execution_date and logical_date from arguments in api_connexion #43678

Closed

Conversation

sunank200
Copy link
Collaborator

This PR removes the execution_date and logical_date arguments from api_connexion that are used to retrieve DAG runs, aligning with the broader changes introduced in Airflow 2.2 and preparing for Airflow 3.0. The functions now use run_id as the sole identifier for DAG runs, simplifying the process and eliminating deprecated behaviour.

Motivation:

In Airflow, execution_date has historically been used to distinguish different DAG run instances. However, the introduction of run_id and the DAG run concept in Airflow 2.2 shifts away from using execution_date as an identifier. Continuing to rely on execution_date introduces limitations, such as the inability to handle multiple DAG runs at the same logical time, especially in cases like TriggerDagRunOperator when dynamic runs are generated.

By removing execution_date in favor of run_id, this PR eliminates these limitations. This also removes the unique constraint on execution_date at the database level, paving the way for a cleaner and more flexible scheduling system in Airflow 3.0.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Nov 5, 2024
@sunank200 sunank200 added the legacy api Whether legacy API changes should be allowed in PR label Nov 5, 2024
@potiuk
Copy link
Member

potiuk commented Nov 5, 2024

Are we going to remove it first and then move the API ? I was under the impression we wanted to make it new "while" moving it to the new fast_api framework? But maybe there is a good reason to do it first here?

@Lee-W
Copy link
Member

Lee-W commented Nov 6, 2024

Are we going to remove it first and then move the API ? I was under the impression we wanted to make it new "while" moving it to the new fast_api framework? But maybe there is a good reason to do it first here?

As long as there's no conflict, I think it's ok to do it first 🤔 so that whoever's working on moving the API

@Lee-W Lee-W added the legacy ui Whether legacy UI change should be allowed in PR label Nov 6, 2024
@Lee-W Lee-W self-requested a review November 6, 2024 08:29
@@ -285,7 +285,7 @@ def stats_tags(self) -> dict[str, str]:
return prune_dict({"dag_id": self.dag_id, "run_type": self.run_type})

@property
def logical_date(self) -> datetime:
def logical_date(self):
return self.execution_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 think this is going to be removed in another PR?

ret_data = {}
data["execution_date"] = data["logical_date"]
data["logical_date"] = data["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.

Suggested change
data["logical_date"] = data["logical_date"]

I think we probably don't need this line?

@@ -212,8 +212,8 @@ class SetTaskInstanceStateFormSchema(Schema):
@validates_schema
def validate_form(self, data, **kwargs):
"""Validate set task instance state form."""
if not exactly_one(data.get("execution_date"), data.get("dag_run_id")):
raise ValidationError("Exactly one of execution_date or dag_run_id must be provided")
if not exactly_one(data.get("logical_date"), data.get("dag_run_id")):
Copy link
Member

Choose a reason for hiding this comment

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

Is this still needed after the removal in airflow/api_connexion/openapi/v1.yaml

@@ -173,12 +173,12 @@ def _fetch_dag_runs(
query = query.where(DagRun.updated_at <= updated_at_lte)

total_entries = get_query_count(query, session=session)
to_replace = {"dag_run_id": "run_id", "execution_date": "logical_date"}
to_replace = {"dag_run_id": "run_id", "logical_date": "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.

Do we need to replace it?

@Lee-W Lee-W added the AIP-83 Remove Execution Date Unique Constraint from DAG Run label Nov 6, 2024
@Lee-W
Copy link
Member

Lee-W commented Nov 6, 2024

For these pull requests, we will probably need the legacy ui and legacy api labels to run the CI. Please let me know if you need help adding the labels to the following pull requests. 🙂

@sunank200
Copy link
Collaborator Author

This can be closed rename of execution_date is done as part of logical_date here: 43902

Removal of execution_date is being done in 42404

@sunank200 sunank200 closed this Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-83 Remove Execution Date Unique Constraint from DAG Run area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues legacy api Whether legacy API changes should be allowed in PR legacy ui Whether legacy UI change should be allowed in PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants