-
Notifications
You must be signed in to change notification settings - Fork 365
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
Support context.Context #326
Comments
I have the beginning of a PR for context cancelation. One blocking point is the behavior on cancel. Should it become an abandon request? This one was not implemented here last time I checked. |
Definitely would be a nice feature, I am using context.Context almost everywhere and have some ugly wrapper around the ldap client. Support for it would be great. |
SUGGESTION: How about enhancing the Example: // Add performs the given AddRequest
func (l *Conn) Add(addRequest *AddRequest) error
// AddWithContext performs the given AddRequest with a context, the context controls the entire lifetime of a request and its response
func (l *Conn) AddWithContext(ctx context.Context, addRequest *AddRequest) error
// Search performs the given search request
func (l *Conn) Search(searchRequest *SearchRequest) (*SearchResult, error)
// SearchWithContext performs the given search request with a context, the context controls the entire lifetime of a request and its response
func (l *Conn) SearchWithContext(ctx context.Context, searchRequest *SearchRequest) (*SearchResult, error)
... This would ensure API compatability to the old methods and would not cause breaking changes for an upcoming patch like this. There would be another solution I thought of but it would introduce BREAKING CHANGES, which are most likely not wanted. |
I like this suggestion in principle. I'm unclear about actual implementation of the timeout and cancellation logic, but there should be ample examples and suggestions in other projects. |
I'll knock out an attempt at an initial implementation |
I can join if you want 👍 |
Here's a first cut: #406 Not mirrored into v3 (yet), just bashing out what the basic API would look like. Probably still buggy. |
The same concept has been implemented by other libraries, each method has an equivalent with the option to provide a context. I would follow the same scheme here |
This module currently lacks any support for context.Context, so it can't inherit conventional timeout and cancellation semantics. That seems worth adding.
The text was updated successfully, but these errors were encountered: