-
Notifications
You must be signed in to change notification settings - Fork 42
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
context.Context support is missing #98
context.Context support is missing #98
Comments
Hello @brenol, Thank you for taking the time to report this feature request! This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog. With best regards, Elmer |
You might want to also add a note in the README that the API is going to change, unless you are going to double up every single endpoint like AWS does (CreateMessage and CreateMessageWithContext, etc.) |
Hi @thinkingserious , I'd be willing to take this on if you can direct me on how you want the API's evolved. I envision that both the client's and API packages are going to need meaningful changes to accommodate this: Approach 1 - break API's:
Approach 2 - deprecate and expose API's w/ context.
I'm generally assuming y'all would prefer adding methods rather than breaking. |
Hi @natebrennand, thank you for your suggestions! We like the approach 2, if you could submit a PR with a proof of concept, we will work on getting the changes inside the generator. |
I've got PR's open to update the underlying client and the SDK to support this: |
Fixes #98 This PR uses the new alternative base client interfaces, `BaseClientWithCtx` and `RequestHandlerWithCtx`, that allow `context.Context` objects to be provided to every API request. This will allow the Twilio SDK to support context cancellations, and for custom HTTP clients to access the context while executing the request.
Am I missing something? This is marked as done but version 1.7.1 of the API, released 5 days ago, does not seem to have support for context. For example, the signature of CreateVerification looks like this: func (c *ApiService) CreateVerification(ServiceSid string, params *CreateVerificationParams) (*VerifyV2Verification, error) whereas I'm expecting it to look like this: func (c *ApiService) CreateVerification(ctx context.Context, ServiceSid string, params *CreateVerificationParams) (*VerifyV2Verification, error) |
Hi. I'd like to inquire about any updates regarding this issue. |
Any update here? |
Issue Summary
Not really a bug, but a missing functionality. Expected to see methods with
context.Context
support. However, currently there is no support for it and this makes it not being possible to do proper cancellation with twilio's API.Code Snippet
N/A
Exception/Log
N/A
Technical details:
The text was updated successfully, but these errors were encountered: