Skip to content

Moving EmptyOperator to standard provider - #46231

Merged
amoghrajesh merged 15 commits into
apache:mainfrom
astronomer:AIP72-move-empty-operator
Feb 4, 2025
Merged

Moving EmptyOperator to standard provider#46231
amoghrajesh merged 15 commits into
apache:mainfrom
astronomer:AIP72-move-empty-operator

Conversation

@amoghrajesh

@amoghrajeshamoghrajesh commented Jan 29, 2025

Copy link
Copy Markdown
Contributor

Basic dag tested:

from airflow import DAG
from airflow.providers.standard.operators.empty import EmptyOperator
from datetime import datetime
with DAG(
dag_id="empty_operator_example",
schedule=None,
tags=["empty"],
start_date=datetime(2024, 1, 1),
catchup=False,
) as dag:
start = EmptyOperator(task_id="start")
end = EmptyOperator(task_id="end")
start >> end

^ 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.

@eladkal

Copy link
Copy Markdown
Contributor

It needs newsfragment as breaking change for Airflow 3

@eladkaleladkal added the airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes label Jan 29, 2025
@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

@eladkal aren't the other ones for example: bash operator breaking in that case too? #42252

Comment threadnewsfragments/46231.significant.rst
@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

@potiuk it'd be nice if you can take a look on this too, since you were involved in the past standard operator moves

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

Overall LGTM. Added few comments

Comment threadtests_common/pytest_plugin.py
Comment threadtests/serialization/test_dag_serialization.py Outdated
@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

@eladkal@ashb got a green CI finally. Can you take a look at this when you have some time?

@Lee-W
Lee-W self-requested a review February 3, 2025 00:51
@Lee-W

Lee-W commented Feb 3, 2025

Copy link
Copy Markdown
Member

@eladkal aren't the other ones for example: bash operator breaking in that case too? #42252

Yep, we'll need to create a newsfragment for that.

@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

@vincbeck we will be adding the migration rules for this as requested. @sunank200 / @Lee-W will be assisting with that.

@eladkal

Copy link
Copy Markdown
Contributor

May I know what is test_zip.zip and why do we need it?

You can deploy dag in zip files.
I assume that the example dag in that zip contains EmptyOperator

@Lee-W

Lee-W commented Feb 3, 2025

Copy link
Copy Markdown
Member

May I know what is test_zip.zip and why do we need it?

You can deploy dag in zip files. I assume that the example dag in that zip contains EmptyOperator

@amoghrajesh Is it possible for us to zip it through script instead of just uploading a zip file? It's hard to know what's actually inside the zip

@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

May I know what is test_zip.zip and why do we need it?

You can deploy dag in zip files. I assume that the example dag in that zip contains EmptyOperator

It does yeah, it contains a reference to the older EmptyOperator path, i updated that

@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

May I know what is test_zip.zip and why do we need it?

You can deploy dag in zip files. I assume that the example dag in that zip contains EmptyOperator

@amoghrajesh Is it possible for us to zip it through script instead of just uploading a zip file? It's hard to know what's actually inside the zip

Hmm, its been done this way historically too, I guess we can but I can do it in a follow up if thats ok? I will list the zip changes here:

(airflow) ➜ dags git:(AIP72-move-empty-operator) ✗ ls -lR test_zip total 16
-rw-r--r-- 1 amoghdesai staff 566 Jun 16 2018 file_no_airflow_dag.py
-rw-r--r-- 1 amoghdesai staff 1171 Jan 30 12:50 test_zip.py
drwxr-xr-x 4 amoghdesai staff 128 Apr 13 2016 test_zip_module
test_zip/test_zip_module:
total 8
-rw-r--r-- 1 amoghdesai staff 0 Apr 13 2016 __init__.py
-rw-r--r-- 1 amoghdesai staff 642 Apr 13 2016 test.py

@Lee-W

Lee-W commented Feb 3, 2025

Copy link
Copy Markdown
Member

May I know what is test_zip.zip and why do we need it?

You can deploy dag in zip files. I assume that the example dag in that zip contains EmptyOperator

@amoghrajesh Is it possible for us to zip it through script instead of just uploading a zip file? It's hard to know what's actually inside the zip

Hmm, its been done this way historically too, I guess we can but I can do it in a follow up if thats ok? I will list the zip changes here:

(airflow) ➜ dags git:(AIP72-move-empty-operator) ✗ ls -lR test_zip total 16
-rw-r--r-- 1 amoghdesai staff 566 Jun 16 2018 file_no_airflow_dag.py
-rw-r--r-- 1 amoghdesai staff 1171 Jan 30 12:50 test_zip.py
drwxr-xr-x 4 amoghdesai staff 128 Apr 13 2016 test_zip_module
test_zip/test_zip_module:
total 8
-rw-r--r-- 1 amoghdesai staff 0 Apr 13 2016 __init__.py
-rw-r--r-- 1 amoghdesai staff 642 Apr 13 2016 test.py

Yep, I know it's done this way previously. If this is blocking something, we can create an issue for tracking this and do it in follow up PRs. Thanks!

@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

Yeah, will pick it up @Lee-W.
It isn't related to this change, so better to follow up

@Lee-W

Lee-W commented Feb 4, 2025

Copy link
Copy Markdown
Member

@amoghrajesh Sorry for the CI error. The fix is created #46404

@amoghrajesh

Copy link
Copy Markdown
ContributorAuthor

@amoghrajesh Sorry for the CI error. The fix is created #46404

No problem, thanks! I was wondering what the issue was!

@Lee-W

Lee-W commented Feb 4, 2025

Copy link
Copy Markdown
Member

Feel free to resolve my comments. I think we're close to merge 🙂

@amoghrajesh
amoghrajesh merged commit e6ea670 into apache:mainFeb 4, 2025
@amoghrajesh
amoghrajesh deleted the AIP72-move-empty-operator branch February 4, 2025 06:34
@Lee-WLee-W mentioned this pull request Feb 5, 2025
2 tasks
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
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:lineagearea:production-imageProduction image improvements and fixeskind:documentationprovider:common-ioprovider:common-sqlprovider:edgeEdge Executor / Worker (AIP-69) / edge3provider:openlineageAIP-53

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@amoghrajesh@eladkal@Lee-W@sunank200@pierrejeambrun@vincbeck