naruto-lgtm commented on PR #68662: URL: https://github.com/apache/airflow/pull/68662#issuecomment-4756842424
Agreed, that's the right line to draw. I traced where these blobs actually flow first: the cross-boundary control-flow signals (reschedule, deferral, downstream-skip, awaiting-input) already travel as their own comms messages (RescheduleTask, DeferTask, SkipDownstreamTasks, ...) and get reconstructed explicitly in the task runner. So the AIRFLOW_EXC_SER/BASE_EXC_SER branch in BaseSerialization isn't what carries them across the worker/supervisor boundary. What it actually catches is exceptions that happen to be nested inside some other serialized value (XCom, trigger/next kwargs), which is exactly the general-case behaviour you want gone from a module that runs in so many processes. Plan I'd go with: - Drop both encode branches (the AirflowException-with-serialize one and the KeyError/AttributeError one) and the matching decode branch from BaseSerialization, so no process running general serialization can import-and-call an arbitrary name from a payload anymore. - For anything that genuinely still needs a real exception instance back, add a small dedicated helper with a fixed name->class allowlist and reconstruct from that map only, no import_string, kept out of the general framework. One thing I want to confirm before wiring it up: from the call-site sweep I don't see a runtime path that depends on getting an exception instance back out of general serialization once the comms messages are accounted for. So my inclination is to remove it outright rather than reintroduce even a narrow allowlist, and only add the allowlisted helper if there's a specific round-trip that needs it. Does that match what you had in mind, or is there a case you know of that I should keep behind the allowlist? I'll split it so the framework removal and any targeted replacement land as separate commits. -- 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]
