Lee-W commented on code in PR #44908:
URL: https://github.com/apache/airflow/pull/44908#discussion_r1887987399
##########
tests/cli/commands/remote_commands/test_config_command.py:
##########
@@ -232,3 +235,182 @@ def
test_should_raise_exception_when_option_is_missing(self, caplog):
self.parser.parse_args(["config", "get-value", "missing-section",
"dags_folder"])
)
assert "section/key [missing-section/dags_folder] not found in config"
in caplog.text
+
+
+class TestConfigLint:
+ @pytest.mark.parametrize("removed_config", config_command.CONFIGS_CHANGES)
+ def test_lint_detects_removed_configs(self, removed_config):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+
config_command.lint_config(cli_parser.get_parser().parse_args(["config",
"lint"]))
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+ normalized_message = re.sub(r"\s+", " ",
removed_config.message.strip())
+
+ assert normalized_message in normalized_output
+
+ @pytest.mark.parametrize(
+ "section, option, suggestion",
+ [
+ (
+ "core",
+ "check_slas",
+ "The SLA feature is removed in Airflow 3.0, to be replaced
with Airflow Alerts in future",
+ ),
+ (
+ "core",
+ "strict_asset_uri_validation",
+ "Asset URI with a defined scheme will now always be validated
strictly, raising a hard error on validation failure.",
+ ),
+ (
+ "logging",
+ "enable_task_context_logger",
+ "Remove TaskContextLogger: Replaced by the Log table for
better handling of task log messages outside the execution context.",
+ ),
+ ],
+ )
+ def test_lint_with_specific_removed_configs(self, section, option,
suggestion):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+
config_command.lint_config(cli_parser.get_parser().parse_args(["config",
"lint"]))
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+
+ expected_message = f"Removed deprecated `{option}` configuration
parameter from `{section}` section."
+ assert expected_message in normalized_output
+
+ if suggestion:
Review Comment:
It seems the suggestion always exists. I think we can remove this `if` black
##########
tests/cli/commands/remote_commands/test_config_command.py:
##########
@@ -232,3 +235,182 @@ def
test_should_raise_exception_when_option_is_missing(self, caplog):
self.parser.parse_args(["config", "get-value", "missing-section",
"dags_folder"])
)
assert "section/key [missing-section/dags_folder] not found in config"
in caplog.text
+
+
+class TestConfigLint:
+ @pytest.mark.parametrize("removed_config", config_command.CONFIGS_CHANGES)
+ def test_lint_detects_removed_configs(self, removed_config):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+
config_command.lint_config(cli_parser.get_parser().parse_args(["config",
"lint"]))
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+ normalized_message = re.sub(r"\s+", " ",
removed_config.message.strip())
+
+ assert normalized_message in normalized_output
+
+ @pytest.mark.parametrize(
+ "section, option, suggestion",
+ [
+ (
+ "core",
+ "check_slas",
+ "The SLA feature is removed in Airflow 3.0, to be replaced
with Airflow Alerts in future",
+ ),
+ (
+ "core",
+ "strict_asset_uri_validation",
+ "Asset URI with a defined scheme will now always be validated
strictly, raising a hard error on validation failure.",
+ ),
+ (
+ "logging",
+ "enable_task_context_logger",
+ "Remove TaskContextLogger: Replaced by the Log table for
better handling of task log messages outside the execution context.",
+ ),
+ ],
+ )
+ def test_lint_with_specific_removed_configs(self, section, option,
suggestion):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+
config_command.lint_config(cli_parser.get_parser().parse_args(["config",
"lint"]))
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+
+ expected_message = f"Removed deprecated `{option}` configuration
parameter from `{section}` section."
+ assert expected_message in normalized_output
+
+ if suggestion:
+ assert suggestion in normalized_output
+
+ def test_lint_specific_section_option(self):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+ config_command.lint_config(
+ cli_parser.get_parser().parse_args(
+ ["config", "lint", "--section", "core", "--option",
"check_slas"]
+ )
+ )
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+
+ assert (
+ "Removed deprecated `check_slas` configuration parameter from
`core` section."
+ in normalized_output
+ )
+
+ def test_lint_with_invalid_section_option(self):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=False):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+ config_command.lint_config(
+ cli_parser.get_parser().parse_args(
+ ["config", "lint", "--section", "invalid_section",
"--option", "invalid_option"]
+ )
+ )
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+
+ assert "No issues found in your airflow.cfg." in normalized_output
+
+ def test_lint_detects_multiple_issues(self):
+ with mock.patch(
+ "airflow.configuration.conf.has_option",
+ side_effect=lambda s, o: o in ["check_slas",
"strict_asset_uri_validation"],
+ ):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+
config_command.lint_config(cli_parser.get_parser().parse_args(["config",
"lint"]))
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+
+ assert (
+ "Removed deprecated `check_slas` configuration parameter from
`core` section."
+ in normalized_output
+ )
+ assert (
+ "Removed deprecated `strict_asset_uri_validation` configuration
parameter from `core` section."
+ in normalized_output
+ )
+
+ @pytest.mark.parametrize(
+ "removed_configs",
+ [
+ [
+ (
+ "core",
+ "check_slas",
+ "The SLA feature is removed in Airflow 3.0, to be replaced
with Airflow Alerts in future",
+ ),
+ (
+ "core",
+ "strict_asset_uri_validation",
+ "Asset URI with a defined scheme will now always be
validated strictly, raising a hard error on validation failure.",
+ ),
+ (
+ "logging",
+ "enable_task_context_logger",
+ "Remove TaskContextLogger: Replaced by the Log table for
better handling of task log messages outside the execution context.",
+ ),
+ ],
+ [
+ ("webserver", "allow_raw_html_descriptions", ""),
+ ("webserver", "session_lifetime_days", "Please use
`session_lifetime_minutes`."),
+ ],
+ ],
+ )
+ def test_lint_detects_multiple_removed_configs(self, removed_configs):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
+ with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+
config_command.lint_config(cli_parser.get_parser().parse_args(["config",
"lint"]))
+
+ output = temp_stdout.getvalue()
+
+ normalized_output = re.sub(r"\s+", " ", output.strip())
+
+ for section, option, suggestion in removed_configs:
+ expected_message = (
+ f"Removed deprecated `{option}` configuration parameter from
`{section}` section."
+ )
+ assert expected_message in normalized_output
+
+ if suggestion:
+ assert suggestion in normalized_output
+
+ @pytest.mark.parametrize(
+ "renamed_configs",
+ [
+ # Case 1: Renamed configurations within the same section
+ [
+ ("core", "non_pooled_task_slot_count", "core",
"default_pool_task_slot_count"),
+ ("scheduler", "processor_poll_interval", "scheduler",
"scheduler_idle_sleep_time"),
+ ],
+ # Case 2: Renamed configurations across sections
+ [
+ ("admin", "hide_sensitive_variable_fields", "core",
"hide_sensitive_var_conn_fields"),
+ ("core", "worker_precheck", "celery", "worker_precheck"),
+ ],
+ ],
+ )
+ def test_lint_detects_renamed_configs(self, renamed_configs):
+ with mock.patch("airflow.configuration.conf.has_option",
return_value=True):
Review Comment:
I would suggest we add one test case that mocks the env var
--
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]
