naruto-lgtm commented on PR #68662: URL: https://github.com/apache/airflow/pull/68662#issuecomment-6057410464
#68511 landed while this was sitting and closed the actual hole: both branches now resolve against already-loaded classes and a fixed builtin allowlist, with no `import_string` on a payload-supplied name. So the original reason for this PR is gone. I've dropped those commits and rebased, which also takes out the `str(BaseException(...))` line flagged above, since the legacy-decode shim it belonged to went with them. What survives is the one thing #68511 didn't touch: the `LazySelectSequence` branch recurses with `cls.serialize(list(var))` and drops `strict`, so an unserializable element inside one of those gets stringified instead of raising under `strict=True`. That's also the answer to the `strict` question upthread, and why the guard test never caught it. It was breaking out of its AST walk on `var.serialize()` in the exception encode branch, so it only ever validated 2 of the 8 recursions and never reached that line. Walking past instead of breaking makes it cover the method and flag the missing kwarg on its own. On removing exception ser/deser from the general framework: I've left that out rather than carry it here. #68511 deliberately kept that path working, including the pre-3.2.0 re-export handling in `_resolve_airflow_exception` and the tests around it, and deleting that two months after it merged is a bigger call than I should make on my own in a stale PR. Happy to do it as a separate PR if it's still wanted. --- Drafted-by: Claude Code (Opus 5) (no human review before posting) -- 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]
