Skip to content

Skip DockerOperator task when it returns a provided exit code#28996

Merged
potiuk merged 6 commits intoapache:mainfrom
hussein-awala:feat/docker_operator_skip_exit_code
Jan 18, 2023
Merged

Skip DockerOperator task when it returns a provided exit code#28996
potiuk merged 6 commits intoapache:mainfrom
hussein-awala:feat/docker_operator_skip_exit_code

Conversation

@hussein-awala
Copy link
Member

closes: #28951

^ Add meaningful description above
Skip DockerOperator task when the container returns the exit code skip_exit_code provided as an argument.

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.

log_opts_max_size: str | None = None,
log_opts_max_file: str | None = None,
ipc_mode: str | None = None,
skip_exit_code: int = 99,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question. Should we actually provide any default value for that? Or is it user responsibility?
In theory some current users pipelines might return 99 status code

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, I just used the same default value used in the BashOperator

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it make sense to copy default value from BashOperator.

Copy link
Contributor

Choose a reason for hiding this comment

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

And also we need to specify annotation as int | None it is not raise any static checks because we have default value 99 however user might provide None and this is expected value in execute

({"skip_exit_code": None}, 99, AirflowException),
]
)
def test_skip(self, extra_kwargs, actual_exit_code, expected_exc):
Copy link
Contributor

@Taragolis Taragolis Jan 17, 2023

Choose a reason for hiding this comment

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

Would be nice also add this kind of test to docker decorator

Right now docker decorator tests run under non-mocked environment

hussein-awala and others added 2 commits January 18, 2023 20:22
Co-authored-by: Andrey Anshin <Andrey.Anshin@taragol.is>
Co-authored-by: Andrey Anshin <Andrey.Anshin@taragol.is>
Copy link
Contributor

@Taragolis Taragolis left a comment

Choose a reason for hiding this comment

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

Nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a way to skip Docker Operator task

4 participants