-
Notifications
You must be signed in to change notification settings - Fork 22
Incorporate context.Context into SourceManager #159
Comments
SourceManager
I can tackle this if it's okay with you 😄 |
Oh man, that'd be...amazing. 🎉 😄 🎉 But, the refactor here is likely to amount to a rewrite of significant portions of the SourceManager system. It's daunting enough that I'm worried about it turning into one of those dangerous, neverendering refactors...and I wrote the damn thing. Don't get me wrong - I REALLY want other people to take ownership over parts of gps. (So, SO much). But this might not be the best place to start, as fully accomplishing it is likely to be the final step in a series of changes, including #84, #130, #150, and half a dozen more I haven't even had time to write up yet. #84 is probably most directly related to this, but is more narrowly focused, and wouldn't be as affected by this bigger refactor I'm picturing. Maybe you could start by looking at that? |
Yeah, totally, I'll start with #84. I've been taking a look at |
(I'm now working on this in #196) |
This issue was moved to golang/dep#423 |
We need to start using
context.Context
for handling cancellation-type behaviors within theSourceManager
. There are two parts to this:context.Context
should be passed toNewSourceManager()
, and the cancellation channel there used to replace the exposed signal handling system. (It's still fine to have a helper func that sets up a Context for this purpose, though.)SourceManager
methods that touch disk or network - so, pretty much all of them - also need to take acontext.Context
, so that the caller has the option of controlling timeouts or forcing terminations.There's no way this is feasible without a wider refactor of
*SourceMgr
to use channel-based brokers for all its activity, but that's fine - that absolutely needs to happen anyway. We can do all of that at the same time.re: golang/dep#160
The text was updated successfully, but these errors were encountered: