Lee-W commented on code in PR #57700:
URL: https://github.com/apache/airflow/pull/57700#discussion_r2497177221
##########
airflow-core/src/airflow/api_fastapi/core_api/services/public/variables.py:
##########
@@ -148,9 +148,11 @@ def handle_bulk_update(self, action: BulkUpdateAction,
results: BulkActionRespon
for variable in action.entities:
if variable.key not in update_keys:
continue
- variable = update_orm_from_pydantic(variable.key, variable,
action.update_mask, self.session)
+ variable_py = update_orm_from_pydantic(
Review Comment:
```suggestion
updated_variable = update_orm_from_pydantic(
```
##########
airflow-core/src/airflow/models/pool.py:
##########
@@ -198,9 +197,9 @@ def slots_stats(
)
# calculate queued and running metrics
- for pool_name, state, count in state_count_by_pool:
+ for pool_name, state, count_raw in state_count_by_pool:
Review Comment:
```suggestion
for pool_name, state, decimal_count in state_count_by_pool:
```
maybe `decimal_count`, it expresses why we need to do it better
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/athena.py:
##########
@@ -40,11 +40,15 @@
def query_params_to_string(params: dict[str, str | Collection[str]]) -> str:
result = ""
- for key, value in params.items():
+ for key, value_org in params.items():
Review Comment:
original_value?
##########
airflow-core/tests/unit/plugins/test_plugin_ignore.py:
##########
@@ -77,8 +77,8 @@ def test_find_not_should_ignore_path_regexp(self, tmp_path):
"test_load_sub1.py",
}
ignore_list_file = ".airflowignore"
- for file_path in find_path_from_directory(plugin_folder_path,
ignore_list_file, "regexp"):
- file_path = Path(file_path)
+ for file_path_raw in find_path_from_directory(plugin_folder_path,
ignore_list_file, "regexp"):
Review Comment:
```suggestion
for raw_file_path in find_path_from_directory(plugin_folder_path,
ignore_list_file, "regexp"):
```
non-blocking
##########
airflow-core/tests/unit/core/test_impersonation_tests.py:
##########
@@ -58,9 +58,8 @@ def set_permissions(settings: dict[Path | str, int]):
orig_permissions = []
try:
print(" Change file/directory permissions ".center(72, "+"))
- for path, mode in settings.items():
- if isinstance(path, str):
- path = Path(path)
+ for path_raw, mode in settings.items():
Review Comment:
```suggestion
for raw_path, mode in settings.items():
```
non-blocking
##########
airflow-core/src/airflow/executors/executor_loader.py:
##########
@@ -77,8 +77,8 @@ def _get_executor_names(cls) -> list[ExecutorName]:
executor_names = []
for team_name, executor_names_config in all_executor_names:
executor_names_per_team = []
- for name in executor_names_config:
- if len(split_name := name.split(":")) == 1:
+ for name_raw in executor_names_config:
Review Comment:
```suggestion
for executor_name in executor_names_config:
```
##########
airflow-core/src/airflow/serialization/serde.py:
##########
@@ -365,26 +365,23 @@ def _register():
_stringifiers.clear()
with Stats.timer("serde.load_serializers") as timer:
- for _, name, _ in iter_namespace(airflow.serialization.serializers):
- name = import_module(name)
- for s in getattr(name, "serializers", ()):
- if not isinstance(s, str):
- s = qualname(s)
+ for _, module_name, _ in
iter_namespace(airflow.serialization.serializers):
+ name = import_module(module_name)
+ for attr_s in getattr(name, "serializers", ()):
Review Comment:
```suggestion
for serializers in getattr(name, "serializers", ()):
```
##########
airflow-core/src/airflow/serialization/serde.py:
##########
@@ -365,26 +365,23 @@ def _register():
_stringifiers.clear()
with Stats.timer("serde.load_serializers") as timer:
- for _, name, _ in iter_namespace(airflow.serialization.serializers):
- name = import_module(name)
- for s in getattr(name, "serializers", ()):
- if not isinstance(s, str):
- s = qualname(s)
+ for _, module_name, _ in
iter_namespace(airflow.serialization.serializers):
+ name = import_module(module_name)
Review Comment:
```suggestion
module = import_module(module_name)
```
I think it should be a module instead
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -637,8 +637,8 @@ def _get_dag_assets(
inlets: bool = True,
outlets: bool = True,
) -> Iterable[tuple[str, AssetT]]:
- for task in dag.data["dag"]["tasks"]:
- task = task[Encoding.VAR]
+ for task_raw in dag.data["dag"]["tasks"]:
Review Comment:
```suggestion
for raw_task in dag.data["dag"]["tasks"]:
```
`raw_task` fits my taste better, but non-blocking
--
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]
