Skip to content
PR Quorum
Sign inStart free
← Review gallery
sqlalchemy/sqlalchemy #13435openreviewed 6h ago

Add log_note execution option

by Yijian6+219 410 filesreviewed in 37sView PR on GitHub
needs attentionThe panel surfaced issues that deserve attention before merge.1 finding · 1 high+

Reviewer panel

Correctness
deepseek/deepseek-v4-flash
1 found
Security
deepseek/deepseek-v4-flash
0 found
Architecture
deepseek/deepseek-v4-flash
0 found

Findings

01Hhighconf 100%

Abstract method _get_log_note breaks custom ExecutionContext subclasses

lib/sqlalchemy/engine/interfaces.py:3145

The PR adds _get_log_note as an abstract method on ExecutionContext with no default implementation (raises NotImplementedError). Only DefaultExecutionContext provides an implementation. Any third-party dialect or custom context that subclasses ExecutionContext directly (not DefaultExecutionContext) will raise NotImplementedError when self._echo is True and logging occurs. This is a breaking change for such subclasses and will cause runtime errors in production if logging is enabled. The fix is to provide a default implementation in ExecutionContext that returns "" (empty string) to maintain backward compatibility.

Want this on your own pull requests?
The same reviewer panel runs on every PR — one focused review, no noise.
Start freeLive demo