kaxil commented on code in PR #65222:
URL: https://github.com/apache/airflow/pull/65222#discussion_r3101400064
##########
airflow-core/src/airflow/serialization/definitions/notset.py:
##########
@@ -31,6 +31,16 @@
class ArgNotSet:
"""Sentinel type for annotations, useful when None is not viable."""
+ @staticmethod
+ def serialize() -> str:
+ return "NOTSET"
+
+ def __repr__(self) -> str:
+ return self.__class__.__name__
+
+ def __str__(self) -> str:
+ return self.__class__.__name__
Review Comment:
Apologies for the conflicting guidance -- my earlier suggestion to use
`self.__class__.__name__` was half-right. It solves the subclass-inheritance
concern I originally raised, but it doesn't deliver the stable sentinel string
the PR sets out to produce:
- `repr(NOTSET)` -> `"ArgNotSet"` (not `"NOTSET"`)
- `repr(SET_DURING_EXECUTION)` -> `"SetDuringExecution"` (not `"DYNAMIC (set
during execution)"`)
Copilot's suggestion to delegate to `self.serialize()` is the right fix. It
keeps subclasses polymorphic *and* matches the PR's stated goal:
```python
def __repr__(self) -> str:
return self.serialize()
def __str__(self) -> str:
return self.serialize()
```
Resulting behavior:
- `repr(NOTSET)` -> `"NOTSET"`
- `repr(SET_DURING_EXECUTION)` -> `"DYNAMIC (set during execution)"`
(auto-inherited via the existing `SetDuringExecution.serialize()` override in
`task-sdk/src/airflow/sdk/definitions/_internal/types.py`)
Single source of truth per class, no drift between dunders and `serialize()`.
Test assertions in `test_argnotset_repr_and_str` should flip accordingly:
```python
assert repr(NOTSET) == "NOTSET"
assert str(NOTSET) == "NOTSET"
assert repr(SET_DURING_EXECUTION) == "DYNAMIC (set during execution)"
assert str(SET_DURING_EXECUTION) == "DYNAMIC (set during execution)"
```
--
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]
