diff --git a/airflow-core/newsfragments/52860.significant.rst b/airflow-core/newsfragments/52860.significant.rst deleted file mode 100644 index 5962897ec206d..0000000000000 --- a/airflow-core/newsfragments/52860.significant.rst +++ /dev/null @@ -1,17 +0,0 @@ -Replace API server ``access_logfile`` configuration with ``log_config`` - -The API server configuration option ``[api] access_logfile`` has been replaced with ``[api] log_config`` to align with uvicorn's logging configuration instead of the legacy gunicorn approach. -The new ``log_config`` option accepts a path to a logging configuration file compatible with ``logging.config.fileConfig``, providing more flexible logging configuration for the API server. - -This change also removes the dependency on gunicorn for daemonization, making the API server ``--daemon`` option consistent with other Airflow components like scheduler and triggerer. - -* Types of change - - * [ ] Dag changes - * [x] Config changes - * [ ] API changes - * [ ] CLI changes - * [ ] Behaviour changes - * [ ] Plugin changes - * [ ] Dependency changes - * [ ] Code interface changes diff --git a/airflow-core/src/airflow/cli/cli_config.py b/airflow-core/src/airflow/cli/cli_config.py index eb641e2ea5c0b..97c72fdbbdf74 100644 --- a/airflow-core/src/airflow/cli/cli_config.py +++ b/airflow-core/src/airflow/cli/cli_config.py @@ -618,10 +618,10 @@ def string_lower_type(val): default=conf.get("api", "host"), help="Set the host on which to run the API server", ) -ARG_API_SERVER_LOG_CONFIG = Arg( - ("--log-config",), - default=conf.get("api", "log_config", fallback=None), - help="(Optional) Path to the logging configuration file for the uvicorn server. If not set, the default uvicorn logging configuration will be used.", +ARG_API_SERVER_ACCESS_LOGFILE = Arg( + ("-A", "--access-logfile"), + default=conf.get("api", "access_logfile"), + help="The logfile to store the access log. Use '-' to print to stdout", ) ARG_API_SERVER_APPS = Arg( ("--apps",), @@ -1795,7 +1795,7 @@ class GroupCommand(NamedTuple): ARG_DAEMON, ARG_STDOUT, ARG_STDERR, - ARG_API_SERVER_LOG_CONFIG, + ARG_API_SERVER_ACCESS_LOGFILE, ARG_API_SERVER_APPS, ARG_LOG_FILE, ARG_SSL_CERT, diff --git a/airflow-core/src/airflow/cli/commands/api_server_command.py b/airflow-core/src/airflow/cli/commands/api_server_command.py index 2e0744be5b8dd..6ffc27519f23a 100644 --- a/airflow-core/src/airflow/cli/commands/api_server_command.py +++ b/airflow-core/src/airflow/cli/commands/api_server_command.py @@ -21,16 +21,16 @@ import logging import os import subprocess -import sys import textwrap from collections.abc import Callable from functools import wraps from typing import TYPE_CHECKING, TypeVar import uvicorn +from gunicorn.util import daemonize +from setproctitle import setproctitle from airflow import settings -from airflow.cli.commands.daemon_utils import run_command_with_daemon_option from airflow.exceptions import AirflowConfigException from airflow.typing_compat import ParamSpec from airflow.utils import cli as cli_utils @@ -50,55 +50,6 @@ # more info here: https://github.com/benoitc/gunicorn/issues/1877#issuecomment-1911136399 -def _run_api_server(args, apps: str, num_workers: int, worker_timeout: int, proxy_headers: bool): - """Run the API server.""" - log.info( - textwrap.dedent( - f"""\ - Running the uvicorn with: - Apps: {apps} - Workers: {num_workers} - Host: {args.host}:{args.port} - Timeout: {worker_timeout} - Logfiles: {args.log_file or "-"} - =================================================================""" - ) - ) - # get ssl cert and key filepaths here instead of passing them as arguments to reduce the number of arguments - ssl_cert, ssl_key = _get_ssl_cert_and_key_filepaths(args) - - # setproctitle causes issue on Mac OS: https://github.com/benoitc/gunicorn/issues/3021 - os_type = sys.platform - if os_type == "darwin": - log.debug("Mac OS detected, skipping setproctitle") - else: - from setproctitle import setproctitle - - setproctitle(f"airflow api_server -- host:{args.host} port:{args.port}") - - uvicorn_kwargs = { - "host": args.host, - "port": args.port, - "workers": num_workers, - "timeout_keep_alive": worker_timeout, - "timeout_graceful_shutdown": worker_timeout, - "ssl_keyfile": ssl_key, - "ssl_certfile": ssl_cert, - "access_log": True, - "proxy_headers": proxy_headers, - } - # Only set the log_config if it is provided, otherwise use the default uvicorn logging configuration. - if args.log_config and args.log_config != "-": - # The [api/log_config] is migrated from [api/access_logfile] and [api/access_logfile] defaults to "-" for stdout for Gunicorn. - # So we need to check if the log_config is set to "-" or not; if it is set to "-", we regard it as not set. - uvicorn_kwargs["log_config"] = args.log_config - - uvicorn.run( - "airflow.api_fastapi.main:app", - **uvicorn_kwargs, - ) - - def with_api_apps_env(func: Callable[[Namespace], RT]) -> Callable[[Namespace], RT]: """We use AIRFLOW_API_APPS to specify which apps are initialized in the API server.""" @@ -129,6 +80,7 @@ def api_server(args: Namespace): print(settings.HEADER) apps = args.apps + access_logfile = args.access_logfile or "-" num_workers = args.workers worker_timeout = args.worker_timeout proxy_headers = args.proxy_headers @@ -155,25 +107,41 @@ def api_server(args: Namespace): if args.proxy_headers: run_args.append("--proxy-headers") - if args.log_config and args.log_config != "-": - run_args.extend(["--log-config", args.log_config]) - with subprocess.Popen( run_args, close_fds=True, ) as process: process.wait() else: - run_command_with_daemon_option( - args=args, - process_name="api_server", - callback=lambda: _run_api_server( - args=args, - apps=apps, - num_workers=num_workers, - worker_timeout=worker_timeout, - proxy_headers=proxy_headers, - ), + if args.daemon: + daemonize() + log.info("Daemonized the API server process PID: %s", os.getpid()) + + log.info( + textwrap.dedent( + f"""\ + Running the uvicorn with: + Apps: {apps} + Workers: {num_workers} + Host: {args.host}:{args.port} + Timeout: {worker_timeout} + Logfiles: {access_logfile} + =================================================================""" + ) + ) + ssl_cert, ssl_key = _get_ssl_cert_and_key_filepaths(args) + setproctitle(f"airflow api_server -- host:{args.host} port:{args.port}") + uvicorn.run( + "airflow.api_fastapi.main:app", + host=args.host, + port=args.port, + workers=num_workers, + timeout_keep_alive=worker_timeout, + timeout_graceful_shutdown=worker_timeout, + ssl_keyfile=ssl_key, + ssl_certfile=ssl_cert, + access_log=access_logfile, # type: ignore + proxy_headers=proxy_headers, ) diff --git a/airflow-core/src/airflow/config_templates/config.yml b/airflow-core/src/airflow/config_templates/config.yml index b718e36e7235e..8237be073b04d 100644 --- a/airflow-core/src/airflow/config_templates/config.yml +++ b/airflow-core/src/airflow/config_templates/config.yml @@ -1391,14 +1391,13 @@ api: type: integer example: ~ default: "120" - log_config: + access_logfile: description: | - Path to the logging configuration file for the uvicorn server. - If not set, the default uvicorn logging configuration will be used. + Log files for the api server. '-' means log to stderr. version_added: ~ type: string - example: path/to/logging_config.yaml - default: ~ + example: ~ + default: "-" ssl_cert: description: | Paths to the SSL certificate and key for the api server. When both are diff --git a/airflow-core/src/airflow/configuration.py b/airflow-core/src/airflow/configuration.py index f4182f65c44c5..5fd67f5334341 100644 --- a/airflow-core/src/airflow/configuration.py +++ b/airflow-core/src/airflow/configuration.py @@ -365,7 +365,6 @@ def sensitive_config_values(self) -> set[tuple[str, str]]: ("api", "secret_key"): ("webserver", "secret_key", "3.0.2"), ("api", "enable_swagger_ui"): ("webserver", "enable_swagger_ui", "3.0.2"), ("dag_processor", "parsing_pre_import_modules"): ("scheduler", "parsing_pre_import_modules", "3.0.3"), - ("api", "log_config"): ("api", "access_logfile", "3.0.4"), } # A mapping of new section -> (old section, since_version). diff --git a/airflow-core/tests/unit/cli/commands/_common_cli_classes.py b/airflow-core/tests/unit/cli/commands/_common_cli_classes.py index 3adfb78e69966..37bbca0784a28 100644 --- a/airflow-core/tests/unit/cli/commands/_common_cli_classes.py +++ b/airflow-core/tests/unit/cli/commands/_common_cli_classes.py @@ -33,7 +33,7 @@ console = Console(width=400, color_system="standard") -class _CommonCLIUvicornTestClass: +class _CommonCLIGunicornTestClass: main_process_regexp: str = "process_to_look_for" @pytest.fixture(autouse=True) @@ -49,12 +49,12 @@ def _check_processes(self, ignore_running: bool): # Confirm that nmain procss hasn't been launched. # pgrep returns exit status 1 if no process matched. # Use more specific regexps (^) to avoid matching pytest run when running specific method. - # For instance, we want to be able to do: pytest -k 'uvicorn' + # For instance, we want to be able to do: pytest -k 'gunicorn' airflow_internal_api_pids = self._find_all_processes(self.main_process_regexp) - uvicorn_pids = self._find_all_processes(r"uvicorn: ") - if airflow_internal_api_pids or uvicorn_pids: + gunicorn_pids = self._find_all_processes(r"gunicorn: ") + if airflow_internal_api_pids or gunicorn_pids: console.print("[blue]Some processes are still running") - for pid in uvicorn_pids + airflow_internal_api_pids: + for pid in gunicorn_pids + airflow_internal_api_pids: with suppress(NoSuchProcess): console.print(psutil.Process(pid).as_dict(attrs=["pid", "name", "cmdline"])) console.print("[blue]Here list of processes ends") @@ -63,9 +63,9 @@ def _check_processes(self, ignore_running: bool): for pid in airflow_internal_api_pids: with suppress(NoSuchProcess): psutil.Process(pid).kill() - if uvicorn_pids: - console.print("[yellow]Forcefully killing all uvicorn processes") - for pid in uvicorn_pids: + if gunicorn_pids: + console.print("[yellow]Forcefully killing all gunicorn processes") + for pid in gunicorn_pids: with suppress(NoSuchProcess): psutil.Process(pid).kill() if not ignore_running: diff --git a/airflow-core/tests/unit/cli/commands/test_api_server_command.py b/airflow-core/tests/unit/cli/commands/test_api_server_command.py index 2f3dec30f3dbb..23cc041e24bcd 100644 --- a/airflow-core/tests/unit/cli/commands/test_api_server_command.py +++ b/airflow-core/tests/unit/cli/commands/test_api_server_command.py @@ -24,19 +24,19 @@ from airflow.cli.commands import api_server_command from airflow.exceptions import AirflowConfigException -from unit.cli.commands._common_cli_classes import _CommonCLIUvicornTestClass +from unit.cli.commands._common_cli_classes import _CommonCLIGunicornTestClass console = Console(width=400, color_system="standard") @pytest.mark.db_test -class TestCliApiServer(_CommonCLIUvicornTestClass): +class TestCliApiServer(_CommonCLIGunicornTestClass): main_process_regexp = r"airflow api-server" @pytest.mark.parametrize( "args, expected_command", [ - pytest.param( + ( ["api-server", "--port", "9092", "--host", "somehost", "--dev"], [ "fastapi", @@ -47,9 +47,8 @@ class TestCliApiServer(_CommonCLIUvicornTestClass): "--host", "somehost", ], - id="dev mode with port and host", ), - pytest.param( + ( ["api-server", "--port", "9092", "--host", "somehost", "--dev", "--proxy-headers"], [ "fastapi", @@ -61,31 +60,6 @@ class TestCliApiServer(_CommonCLIUvicornTestClass): "somehost", "--proxy-headers", ], - id="dev mode with port, host and proxy headers", - ), - pytest.param( - [ - "api-server", - "--port", - "9092", - "--host", - "somehost", - "--dev", - "--log-config", - "my_log_config.yaml", - ], - [ - "fastapi", - "dev", - "airflow-core/src/airflow/api_fastapi/main.py", - "--port", - "9092", - "--host", - "somehost", - "--log-config", - "my_log_config.yaml", - ], - id="dev mode with port, host and log config", ), ], ) @@ -160,158 +134,40 @@ def test_api_apps_env(self, args, dev_mode, original_env): # Verify that the environment variable was set and cleaned up correctly mock_environ.__setitem__.assert_has_calls(expected_setitem_calls) - @pytest.mark.parametrize( - "cli_args, expected_additional_kwargs", - [ - pytest.param( + def test_args_to_uvicorn(self, ssl_cert_and_key): + cert_path, key_path = ssl_cert_and_key + + with ( + mock.patch("uvicorn.run") as mock_run, + ): + args = self.parser.parse_args( [ "api-server", "--pid", "/tmp/x.pid", "--ssl-cert", - "ssl_cert_path_placeholder", + str(cert_path), "--ssl-key", - "ssl_key_path_placeholder", + str(key_path), "--apps", "core", - ], - { - "ssl_keyfile": "ssl_key_path_placeholder", - "ssl_certfile": "ssl_cert_path_placeholder", - }, - id="api-server with SSL cert and key", - ), - pytest.param( - [ - "api-server", - "--log-config", - "my_log_config.yaml", - ], - { - "ssl_keyfile": None, - "ssl_certfile": None, - "log_config": "my_log_config.yaml", - }, - id="api-server with log config", - ), - ], - ) - def test_args_to_uvicorn(self, ssl_cert_and_key, cli_args, expected_additional_kwargs): - cert_path, key_path = ssl_cert_and_key - if "ssl_cert_path_placeholder" in cli_args: - cli_args[cli_args.index("ssl_cert_path_placeholder")] = str(cert_path) - expected_additional_kwargs["ssl_certfile"] = str(cert_path) - if "ssl_key_path_placeholder" in cli_args: - cli_args[cli_args.index("ssl_key_path_placeholder")] = str(key_path) - expected_additional_kwargs["ssl_keyfile"] = str(key_path) - - with ( - mock.patch("uvicorn.run") as mock_run, - ): - args = self.parser.parse_args(cli_args) + ] + ) api_server_command.api_server(args) mock_run.assert_called_with( "airflow.api_fastapi.main:app", - **{ - "host": args.host, - "port": args.port, - "workers": args.workers, - "timeout_keep_alive": args.worker_timeout, - "timeout_graceful_shutdown": args.worker_timeout, - "access_log": True, - "proxy_headers": args.proxy_headers, - **expected_additional_kwargs, - }, + host="0.0.0.0", + port=8080, + workers=4, + timeout_keep_alive=120, + timeout_graceful_shutdown=120, + ssl_keyfile=str(key_path), + ssl_certfile=str(cert_path), + access_log="-", + proxy_headers=False, ) - @pytest.mark.parametrize( - "demonize", - [True, False], - ) - @mock.patch("airflow.cli.commands.daemon_utils.TimeoutPIDLockFile") - @mock.patch("airflow.cli.commands.daemon_utils.setup_locations") - @mock.patch("airflow.cli.commands.daemon_utils.daemon") - @mock.patch("airflow.cli.commands.daemon_utils.check_if_pidfile_process_is_running") - @mock.patch("airflow.cli.commands.api_server_command.uvicorn") - def test_run_command_daemon( - self, mock_uvicorn, _, mock_daemon, mock_setup_locations, mock_pid_file, demonize - ): - mock_setup_locations.return_value = ( - mock.MagicMock(name="pidfile"), - mock.MagicMock(name="stdout"), - mock.MagicMock(name="stderr"), - mock.MagicMock(name="INVALID"), - ) - args = self.parser.parse_args( - [ - "api-server", - "--host", - "my-hostname", - "--port", - "9090", - "--workers", - "2", - "--worker-timeout", - "60", - ] - + (["--daemon"] if demonize else []) - ) - mock_open = mock.mock_open() - with mock.patch("airflow.cli.commands.daemon_utils.open", mock_open): - api_server_command.api_server(args) - - mock_uvicorn.run.assert_called_once_with( - "airflow.api_fastapi.main:app", - host="my-hostname", - port=9090, - workers=2, - timeout_keep_alive=60, - timeout_graceful_shutdown=60, - ssl_keyfile=None, - ssl_certfile=None, - access_log=True, - proxy_headers=False, - ) - - if demonize: - assert mock_daemon.mock_calls[:3] == [ - mock.call.DaemonContext( - pidfile=mock_pid_file.return_value, - files_preserve=None, - stdout=mock_open.return_value, - stderr=mock_open.return_value, - umask=0o077, - ), - mock.call.DaemonContext().__enter__(), - mock.call.DaemonContext().__exit__(None, None, None), - ] - assert mock_setup_locations.mock_calls == [ - mock.call( - process="api_server", - pid=None, - stdout=None, - stderr=None, - log=None, - ) - ] - mock_pid_file.assert_has_calls([mock.call(mock_setup_locations.return_value[0], -1)]) - assert mock_open.mock_calls == [ - mock.call(mock_setup_locations.return_value[1], "a"), - mock.call().__enter__(), - mock.call(mock_setup_locations.return_value[2], "a"), - mock.call().__enter__(), - mock.call().truncate(0), - mock.call().truncate(0), - mock.call().__exit__(None, None, None), - mock.call().__exit__(None, None, None), - ] - else: - assert mock_daemon.mock_calls == [] - mock_setup_locations.mock_calls == [] - mock_pid_file.assert_not_called() - mock_open.assert_not_called() - @pytest.mark.parametrize( "ssl_arguments, error_pattern", [ diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 873b93b965ee2..f1106185bcc9e 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -786,7 +786,6 @@ grpc GSoD gsuite Gunicorn -gunicorn gz Gzip gzipped