bolkedebruin commented on code in PR #29355:
URL: https://github.com/apache/airflow/pull/29355#discussion_r1101560326
##########
airflow/models/taskinstance.py:
##########
@@ -470,13 +477,43 @@ class TaskInstance(Base, LoggingMixin):
def __init__(
self,
- task: Operator,
+ task: Operator | None = None,
execution_date: datetime | None = None,
run_id: str | None = None,
state: str | None = None,
map_index: int = -1,
+ ti_dict: dict[str, Any] | None = None,
+ ):
+ """
+ Constructs TaskInstance object.
+
+ Deprecated, prefer to use "from_task" or "deserialize" static methods.
+ """
+ if ti_dict is not None:
+ # Should only be used by deserialize method.
Review Comment:
I don't think this is required anymore?
##########
airflow/models/taskinstance.py:
##########
@@ -532,6 +569,41 @@ def __init__(
# can be changed when calling 'run'
self.test_mode = False
+ @staticmethod
+ def from_task(
+ task: Operator,
+ execution_date: datetime | None = None,
+ run_id: str | None = None,
+ state: str | None = None,
+ map_index: int = -1,
+ ):
+ """
+ Create TaskInstance from task operator.
+
+ :param task: task's Operator object.
+ :param execution_date: Optional execution time of the task.
+ :param run_id: Optional DAG run ID for the task.
+ :param state: Optional state of the task.
+ :param map_index: Optional map index. Defaults to -1 (non-mapped task).
+ """
+ return TaskInstance(task, execution_date, run_id, state, map_index)
+
+ @staticmethod
+ def deserialize(ti_dict: dict[str, Any], version: int) -> TaskInstance:
+ """Deserialize TaskInstance from dictionary."""
+ if version > TaskInstance.__version__:
+ raise TypeError("version too big, dont know hot to deserialize")
Review Comment:
Mmm you have even copied my typo ;-)
##########
airflow/serialization/serialized_objects.py:
##########
@@ -502,6 +504,8 @@ def deserialize(cls, encoded_var: Any) -> Any:
return Dataset(**var)
elif type_ == DAT.SIMPLE_TASK_INSTANCE:
return SimpleTaskInstance(**cls.deserialize(var))
+ elif type_ == DAT.TASK_INSTANCE:
Review Comment:
Am I incorrect in thinking that migration is a 2 line change in
`internal_api_call` and you are not relying on any of the other
serialized_objects (basically DAG)? If so then I would say do not add technical
dept and migrate now. This allows us to call `serialized_objects` as stale and
soon to be deprecated.
Otherwise, keep it and and it to the todo of AIP-44?
##########
airflow/models/taskinstance.py:
##########
@@ -532,6 +569,41 @@ def __init__(
# can be changed when calling 'run'
self.test_mode = False
+ @staticmethod
+ def from_task(
+ task: Operator,
+ execution_date: datetime | None = None,
+ run_id: str | None = None,
+ state: str | None = None,
+ map_index: int = -1,
+ ):
+ """
+ Create TaskInstance from task operator.
+
+ :param task: task's Operator object.
+ :param execution_date: Optional execution time of the task.
+ :param run_id: Optional DAG run ID for the task.
+ :param state: Optional state of the task.
+ :param map_index: Optional map index. Defaults to -1 (non-mapped task).
+ """
+ return TaskInstance(task, execution_date, run_id, state, map_index)
+
+ @staticmethod
+ def deserialize(ti_dict: dict[str, Any], version: int) -> TaskInstance:
+ """Deserialize TaskInstance from dictionary."""
+ if version > TaskInstance.__version__:
+ raise TypeError("version too big, dont know hot to deserialize")
Review Comment:
I suggest clarifying it a bit, by being more explicit (what version did you
get and what do you support as a max)
--
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]
