potiuk commented on PR #65618: URL: https://github.com/apache/airflow/pull/65618#issuecomment-6563118264
Thanks for the latest updates, @karenbraganz. The trigger defaults and the `"failure"` status take care of the backward-compatibility concern. There are still 9 open review threads, and the two marked as blockers are still present at `305f69568f`: - `hooks/sql.pyi`: both `arun` overloads still lack `read_only: bool = ...`, which the trigger passes, so type checking fails against the stub. - `triggers/sql.py`: the full result set still goes through the `TriggerEvent` into `next_kwargs` in the metadata database, with no size limit and no fallback. A bound such as a row or byte limit that fails clearly, with a pointer to `deferrable=False`, would close that gap. The other threads are still open too: the explicit per-hook close and autocommit methods, `supports_*` as class attributes, the guard for `bytes` results you mentioned, checking hook support before deferring, and the base `aget_conn` coverage. Could you push those changes, or reply on the threads you'd rather not change, and then re-request review? --- Drafted-by: Claude Code (Claude Opus 5); reviewed by @potiuk 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]
