jscheffl commented on code in PR #57700:
URL: https://github.com/apache/airflow/pull/57700#discussion_r2495777888
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -2565,40 +2572,43 @@ def _deserialize_dag_internal(
# Note: Context is passed explicitly through method parameters, no
class attributes needed
- for k, v in encoded_dag.items():
- if k == "_downstream_task_ids":
- v = set(v)
- elif k == "tasks":
+ for k_in, v_in in encoded_dag.items():
+ v: Any
+ k = k_in
+ if k_in == "_downstream_task_ids":
+ v = set(v_in)
+ elif k_in == "tasks":
SerializedBaseOperator._load_operator_extra_links =
cls._load_operator_extra_links
tasks = {}
- for obj in v:
+ for obj in v_in:
if obj.get(Encoding.TYPE) == DAT.OP:
deser = SerializedBaseOperator.deserialize_operator(
obj[Encoding.VAR], client_defaults
)
tasks[deser.task_id] = deser
k = "task_dict"
v = tasks
- elif k == "timezone":
- v = cls._deserialize_timezone(v)
- elif k == "dagrun_timeout":
- v = cls._deserialize_timedelta(v)
- elif k.endswith("_date"):
- v = cls._deserialize_datetime(v)
- elif k == "edge_info":
+ elif k_in == "timezone":
+ v = cls._deserialize_timezone(v_in)
+ elif k_in == "dagrun_timeout":
+ v = cls._deserialize_timedelta(v_in)
+ elif k_in.endswith("_date"):
+ v = cls._deserialize_datetime(v_in)
+ elif k_in == "edge_info":
# Value structure matches exactly
- pass
- elif k == "timetable":
- v = decode_timetable(v)
- elif k == "weight_rule":
- v = decode_priority_weight_strategy(v)
- elif k in cls._decorated_fields:
- v = cls.deserialize(v)
- elif k == "params":
- v = cls._deserialize_params_dict(v)
- elif k == "tags":
- v = set(v)
- # else use v as it is
+ v = v_in
+ elif k_in == "timetable":
+ v = decode_timetable(v_in)
+ elif k_in == "weight_rule":
+ v = decode_priority_weight_strategy(v_in)
+ elif k_in in cls._decorated_fields:
+ v = cls.deserialize(v_in)
+ elif k_in == "params":
+ v = cls._deserialize_params_dict(v_in)
+ elif k_in == "tags":
+ v = set(v_in)
+ else:
+ v = v_in
Review Comment:
Hi @uranusjr are you referring to this specific case of serialized objects
(where we could exclude with `# noqa: ...` or is it a general concern?
In rework making the codebase compatible I also had one other point in
https://github.com/apache/airflow/pull/57700/files#diff-94532ce5cd8778da565396e51f54f7a10078725a1f61656615fbb173b7503fceR1818
where making code compliant made me a hard time.
So do you mean in these specific cases we should exclude rule where it makes
code not better? Or do you dislike the rule in general (I think in 90% of cases
it prevents failures).
I can also raise a discussion in devlist if you think we should have a
larger round of forming an opinion on the rule.
--
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]
