Fix airflow celery stop to accept the pid file.#17278
Fix airflow celery stop to accept the pid file.#17278uranusjr merged 1 commit intoapache:mainfrom santosh-d3vpl3x:main
airflow celery stop to accept the pid file.#17278Conversation
|
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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
uranusjr
left a comment
There was a problem hiding this comment.
I think the linter will complain, and we probably need to make sure:
- The custom PID path actually works as we expect.
- The
--pid-less command form still continues to work.
In other words, you should not modify the current test_same_pid_file_is_used_in_start_and_stop test, and instead add a new test that runs the worker in a non-standard PID file location to check the --pid option actually has an effect.
uranusjr
left a comment
There was a problem hiding this comment.
A couple of defensive coding improvements, the logic lgtm!
|
Awesome work, congrats on your first merged pull request! |
|
This bot makes me happy every time. |
Fix `airflow celery stop` to accept the pid file. (apache#17278) Fix isort (apache#17330) Fix failing celery test (apache#17337) This change fixes failing test due to mocking
Background
Airflow celery cli provides a nice interface to launch workers and stop them. This cli setup works very well when there is one worker running on node without a custom PID specified. But if a worker is launched with custom
--pidfile thenairflow celery stopcommand is not capable of stopping the worker. In our setup, we run multiple workers on same node subscribed to different queues. The only way to stop worker is to provideSIGTERM, which isn't always the best strategy as it is not always graceful.Changes made
Add an additional argument to the
airflow celery stopcommand,--pid. If this argument is present then the cli will kill worker associated with pid present in specified pidfile.