shahar1 commented on code in PR #42395:
URL: https://github.com/apache/airflow/pull/42395#discussion_r1769646927
##########
tests/cli/commands/test_fastapi_api_command.py:
##########
@@ -168,3 +169,22 @@ def test_cli_fastapi_api_args(self):
],
close_fds=True,
)
+
+ @pytest.mark.parametrize(
+ "ssl_arguments",
+ [["--ssl-cert", "_.crt", "--ssl-key", "_.key"], ["--ssl-cert",
"_.crt"], ["--ssl-key", "_.key"]],
+ )
+ def test_get_ssl_cert_and_key_filepaths_with_incorrect_usage(self,
ssl_arguments):
+ args = self.parser.parse_args(["fastapi-api"] + ssl_arguments)
+ with pytest.raises(AirflowConfigException):
Review Comment:
All if the raised exceptions are `AirflowConfigException`, I'd add a
`match=...` to ensure that each fails for the expected error.
##########
tests/cli/commands/test_fastapi_api_command.py:
##########
@@ -168,3 +169,22 @@ def test_cli_fastapi_api_args(self):
],
close_fds=True,
)
+
+ @pytest.mark.parametrize(
+ "ssl_arguments",
+ [["--ssl-cert", "_.crt", "--ssl-key", "_.key"], ["--ssl-cert",
"_.crt"], ["--ssl-key", "_.key"]],
+ )
+ def test_get_ssl_cert_and_key_filepaths_with_incorrect_usage(self,
ssl_arguments):
+ args = self.parser.parse_args(["fastapi-api"] + ssl_arguments)
+ with pytest.raises(AirflowConfigException):
+ fastapi_api_command._get_ssl_cert_and_key_filepaths(args)
+
+ def test_get_ssl_cert_and_key_filepaths_with_correct_usage(self, tmp_path):
+ cert_path, key_path = tmp_path / "_.crt", tmp_path / "_.key"
+ cert_path.touch()
+ key_path.touch()
+
+ args = self.parser.parse_args(
+ ["fastapi-api"] + ["--ssl-cert", str(cert_path), "--ssl-key",
str(key_path)]
+ )
+ fastapi_api_command._get_ssl_cert_and_key_filepaths(args)
Review Comment:
Please add an `assert` statement
##########
airflow/cli/commands/fastapi_api_command.py:
##########
@@ -124,6 +125,10 @@ def fastapi_api(args):
"python:airflow.api_fastapi.gunicorn_config",
]
+ ssl_cert, ssl_key = _get_ssl_cert_and_key_filepaths(args)
+ if ssl_cert and ssl_key:
+ run_args += ["--certfile", ssl_cert, "--keyfile", ssl_key]
Review Comment:
Could you please add a test for adding these flags to the command?
--
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]
