Add WithContext methods #192
Conversation
|
LGTM. My code is running good with this branch. |
request.go
Outdated
| // TimeoutContext calculates the required timeout contect required given the passed RequestOpts, and any default opts defined by the BotClient. | ||
| TimeoutContext(opts *RequestOpts) (context.Context, context.CancelFunc) | ||
| // TimeoutContext wraps the existing context with the timeout from the provided RequestOpts, or any default opts defined by the BotClient. | ||
| TimeoutContext(ctx context.Context, opts *RequestOpts) (context.Context, context.CancelFunc) |
There was a problem hiding this comment.
BTW, just curious about this method 😄
What's your consideration to define this method separately?
I think we can pass timeout parameter to https://github.com/PaulSonOfLars/gotgbot/blob/v2.0.0-rc.29/bot.go#L42 to achieve the same effect and even if we need ctx to do other things we can do it in RequestWithContext.
There was a problem hiding this comment.
Well, two reasons:
- bot-wide timeouts
- backwards compatibility
I do agree it would be nice if it wasn't part of it at this point! Might look into it to see if it still needs to be part of the interface though...
There was a problem hiding this comment.
Got. thanks for replying
There was a problem hiding this comment.
Managed to clean it up - you were right, doesn't need to be there after all :)
Awesome, great to hear - thank you! |
What
Add *WithContext methods when dealing with the telegram API, to ensure that we can set the right deadlines where necessary and tell any long-running methods when they should be cancelled.
Addresses #190
Impact
TimeoutContextnow takes an optional parent context too.