Skip to content

Purge existing SLA implementation - #42285

Merged
ferruzzi merged 11 commits into
apache:mainfrom
aws-mwaa:ferruzzi/sla2/0-purge-old-code
Sep 25, 2024
Merged

Purge existing SLA implementation#42285
ferruzzi merged 11 commits into
apache:mainfrom
aws-mwaa:ferruzzi/sla2/0-purge-old-code

Conversation

@ferruzzi

@ferruzziferruzzi commented Sep 17, 2024

Copy link
Copy Markdown
Contributor

As discussed in the Airflow 3.0 dev calls, the first step for the new SLA system is stripping the existing implementation out.

I did this PR in multiple commits to try to make it more transparent. In this PR:

  1. Remove everything related to SLA in the code.
  2. Add back the absolute minimum so that existing DAGs won't throw an exception. @bbovenzi said he can add a UI banner so any DAG which implements an SLA will get a notification that SLA is not working in 3.0, but we don't want all existing DAGs to start throwing exceptions left and right either.
  3. Added a log message to models/dag which triggers if the DAG is init'ed with an "sla_miss_callback"
  4. Went through the docs directory and stripped out all references to the SLA feature.

@ferruzzi
ferruzziforce-pushed the ferruzzi/sla2/0-purge-old-code branch from 5560ba3 to f62927eCompareSeptember 18, 2024 16:15

@o-nikolaso-nikolas 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.

There are is an SLA related configuration that I don't see removed in this PR?
https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#check-slas

Comment threaddocs/apache-airflow/core-concepts/tasks.rst
@jscheffljscheffl added the airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes label Sep 18, 2024

@jscheffljscheffl 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.

Ah, added the breaking label, now CI test fails with exactly the comment I wanted to make - please add a newsfragment for the breaking change
Otherwise - looks good for me.

I am a bit surprised that the carve-out of SLA just gains 1k LoC

@ferruzzi

ferruzzi commented Sep 18, 2024

Copy link
Copy Markdown
ContributorAuthor

I am a bit surprised that the carve-out of SLA just gains 1k LoC

Yeah. It's an odd one. I really expected this to be a much larger surgery with tendrils all over the place. It wasn't as bad as I had feared; 28 files is pretty broad but a solid chunk of them are docs pages which just linked back tot he SLA concept page.

please add a newsfragment for the breaking change

I should have known to do that, sorry. I'll add it.

There are is an SLA related configuration that I don't see removed in this PR?

Yeah, I may have missed that one. I'll make another pass and see what I can find related to "check_slas" and prune it.

Maybe a sentence or two describing this feature as deprecated in 3.0? And that there are plans to add it back in 3.1?

I'll come up with something.

Comment threadnewsfragments/42285.significant.rst Outdated
@ferruzzi
ferruzziforce-pushed the ferruzzi/sla2/0-purge-old-code branch from 224fe96 to 25e1a7fCompareSeptember 19, 2024 21:36
Comment threadairflow/models/baseoperator.py
@ferruzzi
ferruzzi merged commit 55fe75d into apache:mainSep 25, 2024
@ferruzzi
ferruzzi deleted the ferruzzi/sla2/0-purge-old-code branch September 25, 2024 17:14
This was referenced Oct 7, 2024
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
SLA will be reimplemented in either 3.0 or 3.1
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
SLA will be reimplemented in either 3.0 or 3.1
@ferruzziferruzzi mentioned this pull request Nov 14, 2024
@Lee-WLee-W mentioned this pull request Nov 18, 2024
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

airflow3.0:breakingCandidates for Airflow 3.0 that contain breaking changesarea:providersarea:Schedulerincluding HA (high availability) schedulerarea:serializationkind:documentationprovider:amazonAWS/Amazon - related issuesprovider:pagerdutyprovider:slackprovider:smtp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@ferruzzi@ephraimbuddy@romsharon98@o-nikolas@jscheffl@vincbeck