-
Notifications
You must be signed in to change notification settings - Fork 23
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
da: add client context #28
Conversation
Warning Rate Limit Exceeded@tuxcanfly has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 22 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates involve integrating Changes
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 Configration File (
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
=======================================
Coverage 79.31% 79.31%
=======================================
Files 3 3
Lines 174 174
=======================================
Hits 138 138
Misses 28 28
Partials 8 8 ☔ View full report in Codecov by Sentry. |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- da.go (1 hunks)
- proxy/client.go (5 hunks)
- proxy/server.go (4 hunks)
- test/dummy.go (6 hunks)
- test/test_suite.go (6 hunks)
Additional comments: 6
da.go (6)
8-8: The addition of
context.Context
as the first parameter toMaxBlobSize
aligns with Go's standard practice for handling request lifecycles and allows for better control over RPC operations.14-14: The addition of
context.Context
as the first parameter toGet
is a good practice for managing timeouts and cancellations of requests.17-17: Incorporating
context.Context
intoGetIDs
enhances the method's ability to handle request-scoped values and control flow.20-20: Adding
context.Context
toCommit
is essential for context-aware operations, especially for long-running processes that may require cancellation.26-26: The
Submit
method now correctly takes acontext.Context
parameter, allowing clients to manage the operation with respect to timeouts and cancellations.29-29: The
Validate
method's update to includecontext.Context
is a positive change, ensuring that validation operations are context-aware.
c8cfc07
to
7fcb3cd
Compare
cb6c05d
7fcb3cd
to
cb6c05d
Compare
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- da.go (2 hunks)
- proxy/client.go (5 hunks)
- proxy/server.go (4 hunks)
- test/dummy.go (6 hunks)
- test/test_suite.go (6 hunks)
Additional comments: 28
da.go (6)
15-15: Adding
context.Context
as the first parameter toMaxBlobSize
is in line with Go's convention for context usage in interfaces. This change will allow for better request management and cancellation.21-21: The
Get
method now correctly takes acontext.Context
as the first parameter, enabling improved error handling and request cancellation.24-24: The
GetIDs
method has been updated to accept acontext.Context
parameter, which is a standard practice for handling request lifecycles in Go.27-27: The
Commit
method's signature now includescontext.Context
, aligning with Go's context pattern for managing timeouts and cancellations.34-34: The
Submit
method correctly includes acontext.Context
parameter, allowing clients to manage the request lifecycle effectively.37-37: The
Validate
method has been updated to take acontext.Context
as the first parameter, which is consistent with Go's context handling pattern.proxy/server.go (6)
28-28: The
MaxBlobSize
method in the proxy server now correctly passes thecontext.Context
to the underlying DA method.34-34: The
Get
method implementation in the proxy server has been updated to include thecontext.Context
when calling the underlying DA method.39-39: The
GetIDs
method in the proxy server now properly includes thecontext.Context
in its call to the underlying DA method.49-49: The
Commit
method in the proxy server has been correctly updated to pass thecontext.Context
to the DA'sCommit
method.61-61: The
Submit
method in the proxy server now passes thecontext.Context
to the underlying DA method, aligning with the context handling pattern in Go.83-83: The
Validate
method in the proxy server has been updated to include thecontext.Context
when calling the DA'sValidate
method.proxy/client.go (6)
41-41: The
MaxBlobSize
method of theClient
type now correctly accepts acontext.Context
parameter.51-51: The
Get
method of theClient
type has been updated to include acontext.Context
parameter, which is a standard practice in Go for handling request lifecycles.67-67: The
GetIDs
method of theClient
type now takes acontext.Context
parameter, allowing for better request management.78-78: The
Commit
method signature in theClient
type has been correctly updated to include acontext.Context
parameter.92-92: The
Submit
method of theClient
type now includes acontext.Context
parameter, aligning with Go's context handling pattern.114-114: The
Validate
method of theClient
type has been updated to accept acontext.Context
parameter, which is consistent with Go's context handling pattern.test/dummy.go (6)
53-53: The
MaxBlobSize
method of theDummyDA
struct now correctly includes acontext.Context
parameter.58-58: The
Get
method of theDummyDA
struct has been updated to include acontext.Context
parameter, which is a standard practice in Go for handling request lifecycles.82-82: The
GetIDs
method of theDummyDA
struct now takes acontext.Context
parameter, allowing for better request management.94-94: The
Commit
method signature in theDummyDA
struct has been correctly updated to include acontext.Context
parameter.103-103: The
Submit
method of theDummyDA
struct now includes acontext.Context
parameter, aligning with Go's context handling pattern.120-120: The
Validate
method of theDummyDA
struct has been updated to accept acontext.Context
parameter, which is consistent with Go's context handling pattern.test/test_suite.go (4)
44-44: The
BasicDATest
function now correctly creates a context usingcontext.TODO()
for use in DA method calls.106-108: The
CheckErrors
function properly creates a context with a timeout usingcontext.WithTimeout
and uses it in theGet
call.117-117: The
GetIDsTest
function has been updated to create a context usingcontext.TODO()
for use in DA method calls.159-161: The
ConcurrentReadWriteTest
function correctly creates a context with a timeout usingcontext.WithTimeout
for use in DA method calls.
cb6c05d
to
bff650e
Compare
Overview
This PR adds a context to all DA interface methods so that clients can gracefully handle timeouts and/or errors.
Note: This is a breaking change for clients which depend on DA interface.
Checklist
Summary by CodeRabbit
New Features
Refactor
Tests