Respect contexts with timeouts#1948
Conversation
bplaxco
force-pushed
the
ctx-timeout
branch
5 times, most recently
from
a8d7bfd to
8fd9677
Compare
|
@zricethezav I think it should be good to go. I'm going to do a little extra testing but things initially looked good. Sorry for some of the noise in a few unrelated files. I updated the readme and that trimmed out some trailing whitespace on save and |
| select { | ||
| case <-ctx.Done(): | ||
| return ctx.Err() | ||
| default: | ||
| wg.Wait() | ||
| return nil | ||
| } |
There was a problem hiding this comment.
General question: what's the difference between this and doing if ctx.Err() != nil { return ctx.Err() }? Is there any reason why one is better than the other?
There was a problem hiding this comment.
I haven't played with it too deeply, but I've seen most of the examples for how to handle this use the select method. I think it does something special under the covers based on a few quirks I saw but I'd like to experiment with it more on the side to understand exactly what's happening and why select seems to be the way to go instead of just checking for err.
Also I'm not sure if there's ever a situation where ctx may be done but there not be an error 🤷 So I wouldn't be surprised either if it was just a semantic thing.
There was a problem hiding this comment.
No changes here, as far as I can tell?
There was a problem hiding this comment.
Yeah, no changes on notion.go, it was all make format touching whitespace.
|
@bplaxco would you mind resolving conflicts? |
rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
3 participants
Description:
Today contexts with timeouts set aren't fully respected, this is adding some support for that so it's easier to bail out during larger scans.
Also I ran
make formatand it touched up a few other places.Checklist: