-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Docs] clarify connection close behavior of context #1606
Conversation
Updates the README to make it clear that `go-sql-driver/mysql` closes the current connection of the `context.Context` provided to `ExecContext`, `SelectContext`, etc. is cancelled or times out prior to the query returning. As is, that behavior is not clearly documented.
WalkthroughThe update enhances Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
README.md (1)
526-526
: Add a comma after 'Therefore'.A comma is needed after the conjunctive/linking adverb ‘Therefore’ to improve readability.
- Therefore you must change the import path (no `_`): + Therefore, you must change the import path (no `_`):Tools
LanguageTool
[uncategorized] ~526-~526: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ... you need direct access to the package. Therefore you must change the import path (no_
...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (1 hunks)
Additional context used
LanguageTool
README.md
[uncategorized] ~526-~526: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ... you need direct access to the package. Therefore you must change the import path (no_
...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Additional comments not posted (1)
README.md (1)
522-523
: Clear and accurate documentation update.The documentation now correctly states that
SelectContext
,ExecContext
, etc., will close the connection if the provided context is cancelled or times out before the result is received.
Co-authored-by: Inada Naoki <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (1 hunks)
Additional context used
Learnings (1)
Common learnings
Learnt from: shogo82148 PR: go-sql-driver/mysql#1576 File: CHANGELOG.md:1-6 Timestamp: 2024-03-26T14:24:09.629Z Learning: PRs #1562 and #1570 in the go-sql-driver/mysql repository collectively address the same underlying issue related to fixing a race condition when a context is canceled, as documented in the CHANGELOG.md for version 1.8.1.
Additional comments not posted (1)
README.md (1)
522-523
: Approved: Important clarification on context cancellation behavior.The added note clearly explains that the
QueryContext
,ExecContext
, etc., variants provided bydatabase/sql
will cause the connection to be closed if the provided context is canceled or times out before the result is received by the driver. This is an important clarification for users to understand the implications of using context-aware methods.
Description
Updates the README to make it clear that
go-sql-driver/mysql
closes the current connection if thecontext.Context
provided toExecContext
,SelectContext
, etc. is cancelled or times out prior to the query returning.As is, that behavior is not clearly documented.
Checklist
Summary by CodeRabbit
Summary by CodeRabbit