Fix: pass the kb id to the wiki page insert-error warning - #18985
Conversation
The warning that reports partial failures from the bulk page insert in persist_wiki_pages has two placeholders in its format string, %d and %s, but only one argument is supplied. At runtime the logging module cannot format the record, so it writes a "Logging error" traceback to stderr and the warning itself never reaches the log. The knowledge base whose rows were dropped is therefore invisible in exactly the case the line exists to report. Every other logging call in the same function that carries kb=%s passes kb_id_str, seven of them, and that is the value added here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe page-insert warning in ChangesWiki logging fix
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change makes the existing knowledge-base page-insert warning include the knowledge-base ID and emit correctly; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
persist_wiki_pageslogs a warning when the bulk page insert reports errors without raising. Its format string has two placeholders,%dand%s, but only one argument is passed:At runtime the logging module cannot format the record. It catches the
TypeError, writes aLogging errortraceback to stderr, and the warning line is never emitted. So the knowledge base whose rows were dropped is invisible in exactly the situation this line exists to report, and what an operator sees instead is a logging traceback with no kb id in it.Reproduction, on stock Python:
The intended argument is unambiguous:
kb_id_stris in scope, and every other logging call inpersist_wiki_pagesthat carrieskb=%spasses it, seven of them. This adds it, and the diff is one line.The line has been on
mainsince 8379165 (#17931, 6 Aug). Found by runningruff check --select PLE1205,PLE1206over the tree; it is the only hit in 1236 Python files, so there is no wider cleanup hiding behind it. Re-running the check on the patched tree returns clean.