docs: Add warning about ClpKeyValuePairStreamHandler's incompatibility with other handlers. #62
Conversation
…y with other handlers.
WalkthroughA warning was added to the documentation for the Changes
Possibly related PRs
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
50-50: Remove blank line inside blockquotemarkdownlint reports MD028 (“no blank lines inside blockquote”) here. Please remove the empty blockquote line or add a
>if a deliberate break is needed.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
50-50: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
50-50: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (ubuntu-latest, 3.11)
- GitHub Check: test-python37 (ubuntu-22.04, 3.7)
- GitHub Check: test-python37 (macos-13, 3.7)
- GitHub Check: test (ubuntu-latest, 3.8)
- GitHub Check: test (macos-latest, 3.8)
🔇 Additional comments (1)
README.md (1)
46-49: Approve the warning additionThe newly added warning clearly informs users of the limitation:
ClpKeyValuePairStreamHandlermust receive a dictionary in themsgargument and therefore can’t be used concurrently with other handlers that expect a format string. This documentation update addresses the reported confusion well.
Co-authored-by: Lin Zhihao <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
46-50: Refine warning phrasing and remove blank line
The new warning is helpful, but you can improve readability by splitting it into shorter sentences. Also, markdownlint flags a blank line inside the blockquote at line 51 (MD028). Removing that blank line will resolve the lint error. For example:@@ lines 46-50 - > [!WARNING] - > This handler cannot be used with other logging handlers since it requires `msg` (the first - > argument passed to the logging method) to be a dictionary. In contrast, standard handlers - > typically treat `msg` as a format string. In the future, this handler may be moved or reworked to - > avoid confusion. + > [!WARNING] + > This handler cannot be used alongside other Python logging handlers because it requires the + > `msg` argument to be a dictionary instead of a format string. + > We plan to revisit or relocate this handler in a future release to better align with standard + > logging conventions. @@ line 51 + (remove this blank line)Also applies to: 51-51
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
51-51: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (ubuntu-latest, 3.8)
- GitHub Check: test (macos-latest, 3.8)
- GitHub Check: test (ubuntu-latest, 3.11)
- GitHub Check: test-python37 (ubuntu-22.04, 3.7)
- GitHub Check: test-python37 (macos-13, 3.7)
Description
As @junhaoliao reported offline, the ClpKeyValuePairStreamHandler is incompatible with other handlers since it requires the
msgargument passed to the logging methods to be a dictionary whereas other handlers require that it's a format string. After discussing it offline with @LinZhihao-723, we decided to warn users for now and eventually develop a better solution that doesn't violate Python's logging handler conventions.Checklist
breaking change.
Validation performed
Validated the addition to the README looks correct on GitHub.
Summary by CodeRabbit
ClpKeyValuePairStreamHandlerlogging handler about incompatibility with other logging handlers and potential future changes.