Lee-W commented on code in PR #44908:
URL: https://github.com/apache/airflow/pull/44908#discussion_r1885984246
##########
airflow/cli/commands/remote_commands/config_command.py:
##########
@@ -64,3 +64,349 @@ def get_value(args):
print(value)
except AirflowConfigException:
pass
+
+
+class ConfigChange:
+ """Class representing the configuration changes in Airflow 3.0."""
+
+ def __init__(
+ self, section: str, option: str, suggestion: str = "", renamed_to:
tuple[str, str] | None = None
+ ) -> None:
+ """
+ Initialize a ConfigChange instance.
+
+ :param section: The section of the configuration.
+ :param option: The option within the section that is removed or
deprecated.
+ :param suggestion: A suggestion for replacing or handling the removed
configuration.
+ :param renamed_to: The new section and option if the configuration is
renamed.
+ """
+ self.section = section
+ self.option = option
+ self.suggestion = suggestion
+ self.renamed_to = renamed_to
+
+ def get_message(self) -> str:
Review Comment:
I feel we could just make it a property 🤔
##########
airflow/cli/commands/remote_commands/config_command.py:
##########
@@ -64,3 +64,349 @@ def get_value(args):
print(value)
except AirflowConfigException:
pass
+
+
+class ConfigChange:
+ """Class representing the configuration changes in Airflow 3.0."""
+
+ def __init__(
+ self, section: str, option: str, suggestion: str = "", renamed_to:
tuple[str, str] | None = None
+ ) -> None:
+ """
+ Initialize a ConfigChange instance.
+
+ :param section: The section of the configuration.
+ :param option: The option within the section that is removed or
deprecated.
+ :param suggestion: A suggestion for replacing or handling the removed
configuration.
+ :param renamed_to: The new section and option if the configuration is
renamed.
+ """
+ self.section = section
+ self.option = option
+ self.suggestion = suggestion
+ self.renamed_to = renamed_to
+
+ def get_message(self) -> str:
+ """Generate a message for this configuration change."""
+ lint_message = f"Removed deprecated `{self.option}` configuration
parameter from `{self.section}` section. {self.suggestion}"
+
+ if self.renamed_to:
+ new_section, new_option = self.renamed_to
+ rename_message = f" Please use `{new_option}` from section
`{new_section}` instead."
+ lint_message = lint_message + rename_message
Review Comment:
```suggestion
lint_message = f"{lint_message} Please use `{new_option}` from
section `{new_section}` instead."
```
##########
airflow/cli/commands/remote_commands/config_command.py:
##########
@@ -64,3 +64,349 @@ def get_value(args):
print(value)
except AirflowConfigException:
pass
+
+
+class ConfigChange:
+ """Class representing the configuration changes in Airflow 3.0."""
+
+ def __init__(
+ self, section: str, option: str, suggestion: str = "", renamed_to:
tuple[str, str] | None = None
+ ) -> None:
+ """
+ Initialize a ConfigChange instance.
+
+ :param section: The section of the configuration.
+ :param option: The option within the section that is removed or
deprecated.
+ :param suggestion: A suggestion for replacing or handling the removed
configuration.
+ :param renamed_to: The new section and option if the configuration is
renamed.
+ """
+ self.section = section
+ self.option = option
+ self.suggestion = suggestion
+ self.renamed_to = renamed_to
+
+ def get_message(self) -> str:
+ """Generate a message for this configuration change."""
+ lint_message = f"Removed deprecated `{self.option}` configuration
parameter from `{self.section}` section. {self.suggestion}"
+
+ if self.renamed_to:
+ new_section, new_option = self.renamed_to
+ rename_message = f" Please use `{new_option}` from section
`{new_section}` instead."
+ lint_message = lint_message + rename_message
+ return lint_message
+
+
+CONFIGS_CHANGES = [
+ ConfigChange(
+ section="admin",
+ option="hide_sensitive_variable_fields",
+ renamed_to=("core", "hide_sensitive_var_conn_fields"),
+ ),
+ ConfigChange(
+ section="admin",
+ option="sensitive_variable_fields",
+ renamed_to=("core", "sensitive_var_conn_names"),
+ ),
+ ConfigChange(
+ section="core",
+ option="check_slas",
+ suggestion="The SLA feature is removed in Airflow 3.0, to be replaced
with Airflow Alerts in "
+ "Airflow 3.1",
Review Comment:
I'm not sure we want to explicitly mention 3.1 here 🤔 Especially when it's
adding a new feature. I might suggest using a vague description here
##########
airflow/cli/commands/remote_commands/config_command.py:
##########
@@ -64,3 +64,349 @@ def get_value(args):
print(value)
except AirflowConfigException:
pass
+
+
+class ConfigChange:
Review Comment:
Not sure whether making it an `attr` class or `DataClass` would be better 🤔
##########
airflow/cli/commands/remote_commands/config_command.py:
##########
@@ -64,3 +64,349 @@ def get_value(args):
print(value)
except AirflowConfigException:
pass
+
+
+class ConfigChange:
+ """Class representing the configuration changes in Airflow 3.0."""
+
+ def __init__(
+ self, section: str, option: str, suggestion: str = "", renamed_to:
tuple[str, str] | None = None
+ ) -> None:
+ """
+ Initialize a ConfigChange instance.
+
+ :param section: The section of the configuration.
+ :param option: The option within the section that is removed or
deprecated.
+ :param suggestion: A suggestion for replacing or handling the removed
configuration.
+ :param renamed_to: The new section and option if the configuration is
renamed.
+ """
+ self.section = section
+ self.option = option
+ self.suggestion = suggestion
+ self.renamed_to = renamed_to
+
+ def get_message(self) -> str:
+ """Generate a message for this configuration change."""
+ lint_message = f"Removed deprecated `{self.option}` configuration
parameter from `{self.section}` section. {self.suggestion}"
+
+ if self.renamed_to:
+ new_section, new_option = self.renamed_to
+ rename_message = f" Please use `{new_option}` from section
`{new_section}` instead."
+ lint_message = lint_message + rename_message
+ return lint_message
+
+
+CONFIGS_CHANGES = [
+ ConfigChange(
+ section="admin",
+ option="hide_sensitive_variable_fields",
+ renamed_to=("core", "hide_sensitive_var_conn_fields"),
+ ),
+ ConfigChange(
+ section="admin",
+ option="sensitive_variable_fields",
+ renamed_to=("core", "sensitive_var_conn_names"),
+ ),
+ ConfigChange(
+ section="core",
+ option="check_slas",
+ suggestion="The SLA feature is removed in Airflow 3.0, to be replaced
with Airflow Alerts in "
+ "Airflow 3.1",
+ ),
+ ConfigChange(
+ section="core",
+ option="strict_asset_uri_validation",
+ suggestion="Asset URI with a defined scheme will now always be
validated strictly, "
+ "raising a hard error on validation failure.",
+ ),
+ ConfigChange(
+ section="core",
+ option="worker_precheck",
+ renamed_to=("celery", "worker_precheck"),
+ ),
+ ConfigChange(
+ section="core",
+ option="non_pooled_task_slot_count",
+ renamed_to=("core", "default_pool_task_slot_count"),
+ ),
+ ConfigChange(
+ section="core",
+ option="dag_concurrency",
+ renamed_to=("core", "max_active_tasks_per_dag"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_conn",
+ renamed_to=("database", "sql_alchemy_conn"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_engine_encoding",
+ renamed_to=("database", "sql_engine_encoding"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_engine_collation_for_ids",
+ renamed_to=("database", "sql_engine_collation_for_ids"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_pool_enabled",
+ renamed_to=("database", "sql_alchemy_pool_enabled"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_pool_size",
+ renamed_to=("database", "sql_alchemy_pool_size"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_max_overflow",
+ renamed_to=("database", "sql_alchemy_max_overflow"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_pool_recycle",
+ renamed_to=("database", "sql_alchemy_pool_recycle"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_pool_pre_ping",
+ renamed_to=("database", "sql_alchemy_pool_pre_ping"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_schema",
+ renamed_to=("database", "sql_alchemy_schema"),
+ ),
+ ConfigChange(
+ section="core",
+ option="sql_alchemy_connect_args",
+ renamed_to=("database", "sql_alchemy_connect_args"),
+ ),
+ ConfigChange(
+ section="core",
+ option="load_default_connections",
+ renamed_to=("database", "load_default_connections"),
+ ),
+ ConfigChange(
+ section="core",
+ option="max_db_retries",
+ renamed_to=("database", "max_db_retries"),
+ ),
+ ConfigChange(
+ section="api",
+ option="access_control_allow_origin",
+ renamed_to=("api", "access_control_allow_origins"),
+ ),
+ ConfigChange(
+ section="api",
+ option="auth_backend",
+ renamed_to=("api", "auth_backends"),
+ ),
+ ConfigChange(
+ section="logging",
+ option="enable_task_context_logger",
+ suggestion="Remove TaskContextLogger: Replaced by the Log table for
better handling of task log "
+ "messages outside the execution context.",
+ ),
+ ConfigChange(
+ section="metrics",
+ option="metrics_use_pattern_match",
+ ),
+ ConfigChange(
+ section="metrics",
+ option="timer_unit_consistency",
+ suggestion="In Airflow 3.0, the ``timer_unit_consistency`` setting in
the ``metrics`` section is "
Review Comment:
If our cli does not support rst, we probably don't need to use "``"
##########
airflow/cli/commands/remote_commands/config_command.py:
##########
@@ -64,3 +64,349 @@ def get_value(args):
print(value)
except AirflowConfigException:
pass
+
+
+class ConfigChange:
+ """Class representing the configuration changes in Airflow 3.0."""
+
+ def __init__(
+ self, section: str, option: str, suggestion: str = "", renamed_to:
tuple[str, str] | None = None
+ ) -> None:
+ """
+ Initialize a ConfigChange instance.
+
+ :param section: The section of the configuration.
+ :param option: The option within the section that is removed or
deprecated.
+ :param suggestion: A suggestion for replacing or handling the removed
configuration.
+ :param renamed_to: The new section and option if the configuration is
renamed.
+ """
+ self.section = section
+ self.option = option
+ self.suggestion = suggestion
+ self.renamed_to = renamed_to
Review Comment:
We probably could make `renamed_to` a NamedTuple. At first glance, I thought
it was (old_name, new_name), but it turned out to be (section, name)
--
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]
