xBis7 commented on code in PR #71737: URL: https://github.com/apache/airflow/pull/71737#discussion_r3864896049
########## airflow-core/src/airflow/models/dagrun.py: ########## @@ -140,14 +141,41 @@ tracer = trace.get_tracer(__name__) +@dataclass(frozen=True, slots=True, eq=False) +class FinishedTI: + """ + Immutable view of a task instance in a terminal state. + + Keeps the dag_run rescan lightweight and fast: scheduling only reads these columns + off finished task instances, so a fully instrumented TaskInstance per row is + unnecessary. ``task`` is attached from the serialized dag rather than the row. + + ``eq=False`` keeps the same identity-based equality and hashing a TaskInstance has. + + Anything needing a full TaskInstance must re-fetch it (see ``_ensure_type_task_instance``). + """ + + task_id: str + map_index: int + state: str | None + start_date: datetime | None + end_date: datetime | None + task: Operator | None = None + + +# Get the necessary columns from the FinishedTI fields so that the class stays +# the single source of truth. For any needed change, add or remove a field there. Review Comment: To provide some context about the SSOT part, I was adding the `FinishedTI` fields by hand in a query and then I thought, if we later modify `FinishedTI`, we would need to also modify the query which can lead to bugs. So I added this variable to be used for accessing the `FinishedTI` fields. https://github.com/xBis7/airflow/blob/edd3176d01674ecc917ec4270a9a0b5c8fa06eb8/airflow-core/src/airflow/models/dagrun.py#L1490 > Also, makw it clear that the columns are taken from the TI You are referring to this part `getattr(TI,`, right? > I think It should be higher up with the field definitions and remove the part before it that just explains what's done in the line bellow In the `FinishedTI` class docstring? https://github.com/xBis7/airflow/blob/edd3176d01674ecc917ec4270a9a0b5c8fa06eb8/airflow-core/src/airflow/models/dagrun.py#L146-L156 ########## airflow-core/src/airflow/models/dagrun.py: ########## @@ -140,14 +141,41 @@ tracer = trace.get_tracer(__name__) +@dataclass(frozen=True, slots=True, eq=False) +class FinishedTI: + """ + Immutable view of a task instance in a terminal state. + + Keeps the dag_run rescan lightweight and fast: scheduling only reads these columns + off finished task instances, so a fully instrumented TaskInstance per row is + unnecessary. ``task`` is attached from the serialized dag rather than the row. + + ``eq=False`` keeps the same identity-based equality and hashing a TaskInstance has. + + Anything needing a full TaskInstance must re-fetch it (see ``_ensure_type_task_instance``). + """ + + task_id: str + map_index: int + state: str | None + start_date: datetime | None + end_date: datetime | None + task: Operator | None = None + + +# Get the necessary columns from the FinishedTI fields so that the class stays +# the single source of truth. For any needed change, add or remove a field there. Review Comment: To provide some context about the SSOT part, I was adding the `FinishedTI` fields by hand in a query and then I thought, if we later modify `FinishedTI`, we will need to also modify the query which can lead to bugs. So I added this variable to be used for accessing the `FinishedTI` fields. https://github.com/xBis7/airflow/blob/edd3176d01674ecc917ec4270a9a0b5c8fa06eb8/airflow-core/src/airflow/models/dagrun.py#L1490 > Also, makw it clear that the columns are taken from the TI You are referring to this part `getattr(TI,`, right? > I think It should be higher up with the field definitions and remove the part before it that just explains what's done in the line bellow In the `FinishedTI` class docstring? https://github.com/xBis7/airflow/blob/edd3176d01674ecc917ec4270a9a0b5c8fa06eb8/airflow-core/src/airflow/models/dagrun.py#L146-L156 -- 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]
