uranusjr commented on code in PR #59835:
URL: https://github.com/apache/airflow/pull/59835#discussion_r2651010073
##########
airflow-core/src/airflow/task/priority_strategy.py:
##########
@@ -41,46 +42,22 @@ class PriorityWeightStrategy(ABC):
"""
@abstractmethod
- def get_weight(self, ti: TaskInstance):
+ def get_weight(self, ti: TaskInstance) -> int:
"""Get the priority weight of a task."""
- ...
-
- @classmethod
- def deserialize(cls, data: dict[str, Any]) -> PriorityWeightStrategy:
- """
- Deserialize a priority weight strategy from data.
-
- This is called when a serialized DAG is deserialized. ``data`` will be
whatever
- was returned by ``serialize`` during DAG serialization. The default
- implementation constructs the priority weight strategy without any
arguments.
- """
- return cls(**data)
-
- def serialize(self) -> dict[str, Any]:
- """
- Serialize the priority weight strategy for JSON encoding.
-
- This is called during DAG serialization to store priority weight
strategy information
- in the database. This should return a JSON-serializable dict that will
be fed into
- ``deserialize`` when the DAG is deserialized. The default
implementation returns
- an empty dict.
- """
- return {}
+ raise NotImplementedError("must be implemented by a subclass")
def __eq__(self, other: object) -> bool:
"""Equality comparison."""
- if not isinstance(other, type(self)):
- return False
- return self.serialize() == other.serialize()
+ return isinstance(other, type(self))
- def __hash__(self):
- return hash(self.serialize())
+ def __hash__(self) -> int:
+ return hash(None)
Review Comment:
These classes don’t have any attributes so there’s nothing to hash (every
instance is equal to another of the same class). This is one of the most common
ways to produce a whatever value in this case. The other would be `hash(())`;
would that be easier to understand?
Honestly if I’m to implement this from scratch I’d omit both `__eq__` and
`__hash__` in the first place (they are not used anywhere), but since thye have
been implemented, I figured it’s better to keep them.
--
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]
