kaxil commented on code in PR #63783:
URL: https://github.com/apache/airflow/pull/63783#discussion_r2956611925
##########
airflow-core/src/airflow/partition_mappers/temporal.py:
##########
@@ -63,7 +63,7 @@ def deserialize(cls, data: dict[str, Any]) -> PartitionMapper:
)
-class HourlyMapper(_BaseTemporalMapper):
+class ToHourlyMapper(_BaseTemporalMapper):
Review Comment:
DAGs serialized before this rename store the fully-qualified path (e.g.
`"airflow.partition_mappers.temporal.HourlyMapper"`) in the metadata DB. After
this rename, `decode_partition_mapper` calls `import_string()` with that stored
path and it will fail with an `ImportError`.
Should add backward-compat aliases at module level so old serialized paths
still resolve:
```python
# Backward compatibility — old serialized DAGs reference these names
HourlyMapper = ToHourlyMapper
DailyMapper = ToDailyMapper
WeeklyMapper = ToWeeklyMapper
MonthlyMapper = ToMonthlyMapper
QuarterlyMapper = ToQuarterlyMapper
YearlyMapper = ToYearlyMapper
```
Same applies to
`task-sdk/src/airflow/sdk/definitions/partition_mappers/temporal.py` for user
code that imports the old names directly.
##########
airflow-core/tests/unit/serialization/test_serialized_objects.py:
##########
@@ -47,26 +47,26 @@
from airflow.models.xcom_arg import XComArg
from airflow.partition_mappers.identity import IdentityMapper as
CoreIdentityMapper
from airflow.partition_mappers.temporal import (
- DailyMapper as CoreDailyMapper,
- HourlyMapper as CoureHourlyMapper,
- MonthlyMapper as CoreMonthlyMapper,
- QuarterlyMapper as CoreQuarterlyMapper,
- WeeklyMapper as CoreWeeklyMapper,
- YearlyMapper as CoreYearlyMapper,
+ ToDailyMapper as CoreToDailyMapper,
Review Comment:
nit: Pre-existing typo carried forward — `CoureToHourlyMapper` should be
`CoreToHourlyMapper`. Good time to fix since you're renaming everything here.
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -44,18 +44,18 @@
"CronPartitionTimetable",
"DAG",
"DagRunState",
- "DailyMapper",
Review Comment:
The `__all__` list is alphabetically sorted, but the `To*Mapper` entries are
placed where the old `D/H/M/Q/W/Y` entries used to sit, so they're now out of
order (e.g. `ToDailyMapper` sits between `DagRunState` and `DeadlineAlert`).
They should be grouped together in their correct alphabetical position (between
`TaskInstanceState` and `TriggerRule`). Same issue in the
`_lazy_loaded_modules` dict and `__init__.pyi`.
--
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]
