Taragolis commented on PR #35392: URL: https://github.com/apache/airflow/pull/35392#issuecomment-1792254428
I think we bump into the [boolean trap](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) here, seems like logic around `catchup` not a binary, and for resolve it we need to add additional boolean here. And we may find ourselves in a situation when we need to add 4 and 5 logic around catchup, e.g. "catchup only on first" So my proposal, if it possible to try get rid of boolean logic for the catchup, and replace it by Enum or string literal with boolean fallback logic, and deprecation warning about `True` / `False` values for catchup Some simple enum ```python class Catchup(str, Enum): ENABLED = "enabled" DISABLED = "disabled" IGNORE_FIRST = "ignore_first" @classmethod def from_string(cls, value: str | Enum): values = cls.__members__.values() for v in values: if v.value == value: return v msg = ( f"Unsupported Enum value for {cls!r}. Expected one of {', '.join(repr(v.value) for v in values)}, " f"but got {value!r}." ) raise ValueError(msg) def __eq__(self, other): if isinstance(other, str): other = Catchup.from_string(other) elif isinstance(other, bool): return bool(self) is other return self == other def __bool__(self): # Deprecation warning here return self != self.DISABLED ``` However it still might break someone pipeline (I'm not sure) and we need to wait until Airflow 3 for use something different hen boolean here, and for now just add new attribute. Same is valid for already existed `depends_on_past` and `ignore_first_depends_on_past` in BaseOperator -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
