Skip to content

Remove dead AIP-44 trigger-over-BaseSerialization path (DAT.BASE_TRIGGER)#68528

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:remove-dead-aip44-exception-trigger-serialization
Jun 17, 2026
Merged

Remove dead AIP-44 trigger-over-BaseSerialization path (DAT.BASE_TRIGGER)#68528
potiuk merged 1 commit into
apache:mainfrom
potiuk:remove-dead-aip44-exception-trigger-serialization

Conversation

@potiuk

@potiuk potiuk commented Jun 14, 2026

Copy link
Copy Markdown
Member

What

Removes the DAT.BASE_TRIGGER serialization path — encoding/decoding a BaseTrigger instance through BaseSerialization — which is dead code left over from AIP-44's Internal API:

  • the encode branch (isinstance(var, BaseTrigger)), the decode branch (type_ == DAT.BASE_TRIGGER), the DAT.BASE_TRIGGER enum value, and the now-unused BaseTrigger import;
  • TaskDeferred is also dropped from the exception-encode tuple — TaskDeferred(BaseException)'s only role there was carrying a trigger over the AIP-44 RPC.

Generic AirflowException / BASE_EXC_SER serialization is kept — it has live test coverage and an exception can still reach the serializer via XCom / next_kwargs / the ExtendedJSON column.

Why it's dead

The live deferral path never serializes a trigger through BaseSerialization:

  • A task that defers builds a structured DeferTask / TIDeferredStatePayload from defer.trigger.serialize() (the trigger's own classpath + kwargs) — see task_runner._defer_task.
  • The execution API stores Trigger(classpath=<str>, encrypted_kwargs=<blob>) without instantiating anything (routes/task_instances.py); the triggerer later imports the classpath from the DB row.
  • AIP-44's Internal API — the RPC that serialized runtime objects like triggers/exceptions over the wire — has been removed.

DAT.BASE_TRIGGER was referenced only inside serialized_objects.py + enums.py + tests — no external producer or consumer. (BaseTrigger.hash serializes the trigger's kwargs dict, not the trigger, so it never hit this branch.)

Relationship to #67926 / #68511

…GER)

The DAT.BASE_TRIGGER encode/decode (serializing a BaseTrigger instance via BaseSerialization) is a vestige of AIP-44's Internal API. Live deferral uses the structured DeferTask/TIDeferredStatePayload (classpath+kwargs); the execution API stores the classpath opaquely; the triggerer imports it from the DB row. No external producer/consumer of DAT.BASE_TRIGGER remains. Keeps generic AirflowException serialization (live).

Generated-by: Claude Opus 4.8 (1M context)
@potiuk potiuk requested review from ashb and bolkedebruin as code owners June 14, 2026 03:29
@potiuk potiuk marked this pull request as draft June 14, 2026 03:29
@potiuk potiuk marked this pull request as ready for review June 14, 2026 08:37
@potiuk

potiuk commented Jun 14, 2026

Copy link
Copy Markdown
Member Author

OK. @amoghrajesh @ashb -> I believe (and I made a deeper digging and trying to get historical look at it) we can simply remove the "dead code" that is not used any more.

potiuk added a commit to potiuk/airflow that referenced this pull request Jun 15, 2026
…validate before import)

Stacked on apache#68528, which removed the dead AIP-44 BASE_TRIGGER path; the
remaining hardening here is for the exception nodes.

When a serialized DAG carries an exception node (AIRFLOW_EXC_SER /
BASE_EXC_SER), deserialization called import_string on the stored class
path and instantiated whatever it resolved to, without validating the
path or the resulting type.

Add _safe_import_for_deserialize, which validates the class path against a
trusted-namespace prefix list (airflow., plus builtins. for the
BASE_EXC_SER builtin case) before importing, and then confirms the
resolved object is a BaseException subclass. A path outside the trusted
namespaces is rejected without being imported.
@potiuk potiuk merged commit 7e9bdb2 into apache:main Jun 17, 2026
148 checks passed
@potiuk potiuk deleted the remove-dead-aip44-exception-trigger-serialization branch June 17, 2026 02:54
@potiuk potiuk added this to the Airflow 3.3.0 milestone Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants