-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add on kill to ssh #40377
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
Add on kill to ssh #40377
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
|
@potiuk @uranusjr Sorry, I got misguided there with the I can do the same in the ssh hook and return the existing |
Yes. |
Made the change. Wondering if there could be a case where the |
|
Might be a race condition, yes. |
Looks like to close the connection we need to do it in |
|
@potiuk Leaving a reminder here that this PR is ready for review. |
Leaving a reminder that tests are failing. |
Apologies, thought I had seen tests passing before I merged main. Had to do some formatting fixes. Looks like tests are passing now. |
|
Nope. Seems they are still failing |
3c46673 to
95c67ac
Compare
9c88b64 to
fef0eee
Compare
…checks if the ssh conn closes. Updated this test to also check if on_kill has been called
… currently not working - pushing this for help
3abacaa to
0ff05dc
Compare
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @o-nikolas @uranusjr ?
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
This is my first contribution here, so still learning the codebase. The
SSHOperatordoes not have on_kill method that can be used to close a connection when needed. This PR adds aon_killmethod to theSSHOperatorclass.Not sure how extensive the changes should be. The new method should most likely be used in different parts of the codebase that the ssh hook is used.
closes: #40343