uranusjr commented on a change in pull request #17278:
URL: https://github.com/apache/airflow/pull/17278#discussion_r678751397
##########
File path: tests/cli/commands/test_celery_command.py
##########
@@ -142,6 +143,42 @@ def test_same_pid_file_is_used_in_start_and_stop(
celery_command.stop_worker(stop_args)
mock_read_pid_from_pidfile.assert_called_once_with(pid_file)
+ @mock.patch("airflow.cli.commands.celery_command.read_pid_from_pidfile")
+ @mock.patch("airflow.cli.commands.celery_command.worker_bin.worker")
+ @mock.patch("airflow.cli.commands.celery_command.psutil.Process")
+ @conf_vars({("core", "executor"): "CeleryExecutor"})
+ def test_custom_pid_file_is_used_in_start_and_stop(
+ self, mock_celery_worker, mock_read_pid_from_pidfile, mock_process
+ ):
+ pid_file = "custom_test_pid_file"
+
+ # Call worker
+ worker_args = self.parser.parse_args(['celery', 'worker',
'--skip-serve-logs', '--pid', pid_file])
+ celery_command.worker(worker_args)
+ run_mock = mock_celery_worker.return_value.run
+ assert run_mock.call_args
+ _, kwargs = run_mock.call_args
Review comment:
```suggestion
args, kwargs = run_mock.call_args
assert not args
```
Same below
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -183,7 +183,7 @@ def worker(args):
def stop_worker(args):
"""Sends SIGTERM to Celery worker"""
# Read PID from file
- pid_file_path, _, _, _ = setup_locations(process=WORKER_PROCESS_NAME)
+ pid_file_path = args.pid if args.pid else
setup_locations(process=WORKER_PROCESS_NAME)[0]
Review comment:
```suggestion
if args.pid:
pid_file_path = args.pid
else:
pid_file_path, _, _, _ = setup_locations(process=WORKER_PROCESS_NAME)
```
Better to fully unpack the tuple; it ensures the tuple’s length and better
prevents incompatible changes in `setup_locations`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
