feat: plumb through datastore contexts #753
Conversation
providers/providers_manager.go
Outdated
| @@ -125,7 +125,7 @@ func (pm *ProviderManager) run(proc goprocess.Process) { | |||
| // don't really care if this fails. | |||
| _ = gcQuery.Close() | |||
| } | |||
| if err := pm.dstore.Flush(); err != nil { | |||
| if err := pm.dstore.Flush(context.TODO()); err != nil { | |||
There was a problem hiding this comment.
No need for a TODO here. You can pass a ctx to run() from NewProviderManager (the only place where it is invoked).
There was a problem hiding this comment.
I tried this when I wrote this, but run() is not called directly, it is called by goprocess which does not provide access to the ctx, and at that point I noped out because of scope creep.
There was a problem hiding this comment.
I can add an issue to thread those through, and we can follow up later. Trying really hard to minimize scope here, b/c it's already huge...
There was a problem hiding this comment.
func NewProviderManager(ctx context.Context, local peer.ID, ps peerstore.Peerstore, dstore ds.Batching, opts ...Option) (*ProviderManager, error) {
...
pm.proc.Go(func(proc goprocess.Process) { pm.run(ctx, proc) })
...
}
...
func (pm *ProviderManager) run(ctx context.Context, proc goprocess.Process) {
...
}
There was a problem hiding this comment.
I can add an issue to thread those through, and we can follow up later. Trying really hard to minimize scope here, b/c it's already huge...
Adding an issue is fine too. I'll LGTM it then.
There was a problem hiding this comment.
ah that's a fine approach, let me just do it
providers/providers_manager.go
Outdated
| @@ -242,7 +242,7 @@ func writeProviderEntry(dstore ds.Datastore, k []byte, p peer.ID, t time.Time) e | |||
| buf := make([]byte, 16) | |||
| n := binary.PutVarint(buf, t.UnixNano()) | |||
|
|
|||
| return dstore.Put(ds.NewKey(dsk), buf[:n]) | |||
| return dstore.Put(context.TODO(), ds.NewKey(dsk), buf[:n]) | |||
There was a problem hiding this comment.
use the ctx from the AddProvider argument.
providers/providers_manager.go
Outdated
| @@ -302,7 +302,7 @@ func (pm *ProviderManager) getProviderSetForKey(k []byte) (*providerSet, error) | |||
|
|
|||
| // loads the ProviderSet out of the datastore | |||
| func loadProviderSet(dstore ds.Datastore, k []byte) (*providerSet, error) { | |||
| res, err := dstore.Query(dsq.Query{Prefix: mkProvKey(k)}) | |||
| res, err := dstore.Query(context.TODO(), dsq.Query{Prefix: mkProvKey(k)}) | |||
There was a problem hiding this comment.
Same here, if you follow the invocation sequence, it shouldn't be hard to thread through a ctx here, instead of TODO.
providers/providers_manager.go
Outdated
| @@ -329,7 +329,7 @@ func loadProviderSet(dstore ds.Datastore, k []byte) (*providerSet, error) { | |||
| fallthrough | |||
| case now.Sub(t) > ProvideValidity: | |||
| // or just expired | |||
| err = dstore.Delete(ds.RawKey(e.Key)) | |||
| err = dstore.Delete(context.TODO(), ds.RawKey(e.Key)) | |||
providers/providers_manager.go
Outdated
| @@ -341,7 +341,7 @@ func loadProviderSet(dstore ds.Datastore, k []byte) (*providerSet, error) { | |||
| decstr, err := base32.RawStdEncoding.DecodeString(e.Key[lix+1:]) | |||
| if err != nil { | |||
| log.Error("base32 decoding error: ", err) | |||
| err = dstore.Delete(ds.RawKey(e.Key)) | |||
| err = dstore.Delete(context.TODO(), ds.RawKey(e.Key)) | |||
guseggert
force-pushed
the
feat/context
branch
2 times, most recently
from
c1d7c5f
to
669b242
Compare
guseggert
force-pushed
the
feat/context
branch
from
669b242
to
60d8bde
Compare
No description provided.