phanikumv commented on code in PR #64094:
URL: https://github.com/apache/airflow/pull/64094#discussion_r2974108349
##########
airflow-core/tests/unit/serialization/test_serialized_objects.py:
##########
@@ -895,6 +895,50 @@ def test_decode_product_mapper():
assert core_pm.to_downstream("2024-06-15T10:30:00|2024-06-15T10:30:00") ==
"2024-06-15T10|2024-06-15"
+def test_encode_sequence_mapper():
Review Comment:
In test_serialized_objects.py, the new tests are named:
- test_encode_sequence_mapper — should be test_encode_chain_mapper
- test_decode_sequence_mapper — should be test_decode_chain_mapper
This looks like a leftover from the original name before it was renamed to
ChainMapper.
##########
airflow-core/src/airflow/serialization/encoders.py:
##########
@@ -411,6 +413,10 @@ def serialize_partition_mapper(
raise NotImplementedError(f"can not serialize timetable
{type(partition_mapper).__name__}")
return partition_mapper.serialize()
+ @serialize_partition_mapper.register
Review Comment:
The base `serialize_partition_mapper `fallback already calls
`partition_mapper.serialize()`, which does the exact same thing. The registered
handler is dead code — it overrides the fallback, but produces identical
output.
Should be removed (unlike other mappers where the registered handler is the
only serialization path; `ChainMapper.serialize()` makes the handler
redundant).
--
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]
