Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2872193003:
[cipd] delete bad cache files when failing do deploy them. (Closed)
Description[cipd] delete bad cache files when failing to deploy them.
This will let cipd naturally recover from a corrupted cipd cache without
incurring the overhead of actually doing full cache verifications in the
common case.
R=vadimsh@chromium.org
BUG=
Review-Url: https://codereview.chromium.org/2872193003
Committed: https://github.com/luci/luci-go/commit/60ceda867cfa677fa85c4fe8517f6eb31d70d617
Patch Set 1 #Patch Set 2 : some slight improvement #Patch Set 3 : do more better #Patch Set 4 : clock #Patch Set 5 : close files #
Total comments: 26
Patch Set 6 : different approach #Patch Set 7 : fix nit #Patch Set 8 : fix test #Patch Set 9 : fix other test #Patch Set 10 : change test #Patch Set 11 : rebase #
Total comments: 8
Patch Set 12 : fix comments #Patch Set 13 : fix spelling nit #
Total comments: 12
Patch Set 14 : fix things #
Messages
Total messages: 65 (52 generated)
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL
Description was changed from ========== [cipd] delete bad cache files when failing do deploy them. This will let cipd naturally recover from a corrupted cipd cache without incuring the overhead of actually doing full cache verifications in the common case. R=vadimsh@chromium.org BUG= ========== to ========== [cipd] delete bad cache files when failing to deploy them. This will let cipd naturally recover from a corrupted cipd cache without incurring the overhead of actually doing full cache verifications in the common case. R=vadimsh@chromium.org BUG= ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Win Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/3609941693176210)
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client.go File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:650: client.instanceCache.GC(ctx, clock.Now(ctx)) I added this because I thought it would be prudent to give the cache's FileSystem an opportunity to collect trash. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1324: doit := func() error { this is mostly just an indentation of the previous code https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1333: isClosed := false isClosed is added https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1346: doRetry = instanceFile.DeleteIfCached(ctx) This line is new https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1367: } this block is new https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... File cipd/client/cipd/client_test.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client_test.go:497: client := mockClientForFetch(c, tempDir, []local.PackageInstance{inst}) because in the case where we corrupt the cache we need to pull `inst` twice from the mock client. So all the test cases which are supposed to fetch once are now grouped and the corrupt cache test is the odd one out. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client_test.go:597: Convey("FetchAndDeployInstance works with busted cache", func() { this is the new test. thanks Rietveld for making the diff stupid. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client_test.go:619: err = ioutil.WriteFile(cachedFile.Name(), []byte("bananas"), 0666) it turns out that "bananas" is not a valid zip file. who knew? https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... cipd/client/cipd/internal/instancecache.go:87: return f.fs.EnsureFileGone(ctx, f.Name()) ensurefilegone now will move the file to trash if it fails to delete it https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... cipd/client/cipd/internal/instancecache.go:219: if err := c.fs.CleanupTrash(ctx); err != nil { GC now does an opportunistic cleanup of the trash. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/local/... File cipd/client/cipd/local/fs.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/local/... cipd/client/cipd/local/fs.go:259: if f.moveToTrash(ctx, path) == "" { I think this should be generally safe for all callers of EnsureFileGone; if the file gets moved to the trash then it's effectively "gone"
https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client.go File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:339: DeleteIfCached(context.Context) bool should it be called before or after Close? Also, maybe we should 'capture' the context when the object was created, because otherwise this interface looks weird Read(...) // no context Seek(...) // no context Close(...) // no context DeleteIfCached(context) // wtf https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:342: type testInstance interface { can this be moved into _test.go? Can tests just typecast to cachedFile and don't bother with interfaces? https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:650: client.instanceCache.GC(ctx, clock.Now(ctx)) On 2017/05/10 09:19:07, iannucci wrote: > I added this because I thought it would be prudent to give the cache's > FileSystem an opportunity to collect trash. Note that instanceCache does GC every instanceCacheSyncInterval already (8 hours currently). I think it is sufficient. We can reduce it to 4 or something. Doing GC on every invocation is kind of aggressive. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1307: func isZipErr(err error) bool { Please move this to 'local' package, where knowledge about zip is concentrated. local.IsCorruptedPackageErr(err) https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1333: isClosed := false On 2017/05/10 09:19:07, iannucci wrote: > isClosed is added can it completely replace 'owned' then? They appear to be doing almost the same thing. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1347: isClosed = doRetry // disarm the defer because DeleteIfCached closes the file. "DeleteIfCached closes the file": this is not documented https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1366: isClosed = doRetry // disarm the defer because DeleteIfCached closes the file. this logic with booleans is kind of complicated... Perhaps we can simplify it with type Corruptable interface { MarkAsCorrupted() } if isZipErr(err) { c, _ := instanceFile.(Corruptable) if c != nil { c.MarkAsCorrupted() doRetry = true } else { doRetry = false } } Instances marked as corrupted will delete themselves from cache in Close() Wdyt? The only logical change from your code, is that this approach will retry even if deletion from cache fails. I think it's fine, it should be rare. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... cipd/client/cipd/internal/instancecache.go:93: func (c *InstanceCache) Get(ctx context.Context, pin common.Pin, now time.Time) (ret CacheFile, err error) { I think I prefer type CacheFile struct { os.File fs local.FileSystem } func (c *IntstanceCache) Get(...) (*CacheFile, error) { ... } Not a fan of empty struct.
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client.go File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:339: DeleteIfCached(context.Context) bool On 2017/05/10 20:16:53, Vadim Sh. wrote: > should it be called before or after Close? Clarified comment > > Also, maybe we should 'capture' the context when the object was created, because > otherwise this interface looks weird > > Read(...) // no context > Seek(...) // no context > Close(...) // no context > DeleteIfCached(context) // wtf shrug https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:342: type testInstance interface { On 2017/05/10 20:16:53, Vadim Sh. wrote: > can this be moved into _test.go? Can tests just typecast to cachedFile and don't > bother with interfaces? oh yeah good point. Derp. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:650: client.instanceCache.GC(ctx, clock.Now(ctx)) On 2017/05/10 20:16:53, Vadim Sh. wrote: > On 2017/05/10 09:19:07, iannucci wrote: > > I added this because I thought it would be prudent to give the cache's > > FileSystem an opportunity to collect trash. > > Note that instanceCache does GC every instanceCacheSyncInterval already (8 hours > currently). I think it is sufficient. We can reduce it to 4 or something. Doing > GC on every invocation is kind of aggressive. Ok, I'll leave the trash cleanup in GC, but remove this from batch ops. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1307: func isZipErr(err error) bool { On 2017/05/10 20:16:53, Vadim Sh. wrote: > Please move this to 'local' package, where knowledge about zip is concentrated. > > local.IsCorruptedPackageErr(err) Done. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1333: isClosed := false On 2017/05/10 20:16:53, Vadim Sh. wrote: > On 2017/05/10 09:19:07, iannucci wrote: > > isClosed is added > > can it completely replace 'owned' then? They appear to be doing almost the same > thing. Not quite; we still maybe need to close the instance :/ edit: restructured all of this https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/client... cipd/client/cipd/client.go:1347: isClosed = doRetry // disarm the defer because DeleteIfCached closes the file. On 2017/05/10 20:16:53, Vadim Sh. wrote: > "DeleteIfCached closes the file": this is not documented Done. https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/80001/cipd/client/cipd/intern... cipd/client/cipd/internal/instancecache.go:93: func (c *InstanceCache) Get(ctx context.Context, pin common.Pin, now time.Time) (ret CacheFile, err error) { On 2017/05/10 20:16:53, Vadim Sh. wrote: > I think I prefer > > type CacheFile struct { > os.File > fs local.FileSystem > } > > func (c *IntstanceCache) Get(...) (*CacheFile, error) { > ... > } > > Not a fan of empty struct. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/360dd1e405438610) Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/360dd1e3ef9a0210)
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/360de1343ce46b10)
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/360de863795bae10)
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
(ping :P)
iannucci@chromium.org changed reviewers: + dnj@chromium.org
dnj would you mind taking a look? this should have landed a couple days ago :/
Hey sorry - lgtm w/ some comments https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/clien... File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/clien... cipd/client/cipd/client.go:417: // It returns an ReadSeekCloser pointing to the raw package data. The caller nit: returns an InstanceFile https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/inter... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/inter... cipd/client/cipd/internal/instancecache.go:74: os.File Does "os.File" pass by value very well? I think this should be a pointer, since that's how the rest of the API deals with it. https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/local... File cipd/client/cipd/local/reader.go (right): https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/local... cipd/client/cipd/local/reader.go:63: MarkCorrupt(context.Context) Let's name this "CloseAndMarkCorrupt", or override "Close" to have a "corrupt" boolean? https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/local... cipd/client/cipd/local/reader.go:344: // it doesn't check for corruption, but the caller must do so. nit: capital "I"
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/clien... File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/clien... cipd/client/cipd/client.go:417: // It returns an ReadSeekCloser pointing to the raw package data. The caller On 2017/05/21 16:59:21, dnj wrote: > nit: returns an InstanceFile Done. https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/inter... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/inter... cipd/client/cipd/internal/instancecache.go:74: os.File On 2017/05/21 16:59:21, dnj wrote: > Does "os.File" pass by value very well? I think this should be a pointer, since > that's how the rest of the API deals with it. It passes fine, but I agree it's weird... not sure what I was thinking. https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/local... File cipd/client/cipd/local/reader.go (right): https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/local... cipd/client/cipd/local/reader.go:63: MarkCorrupt(context.Context) On 2017/05/21 16:59:21, dnj wrote: > Let's name this "CloseAndMarkCorrupt", or override "Close" to have a "corrupt" > boolean? I was hesitant to provide a non-standard Close, but I think you're right. https://codereview.chromium.org/2872193003/diff/200001/cipd/client/cipd/local... cipd/client/cipd/local/reader.go:344: // it doesn't check for corruption, but the caller must do so. On 2017/05/21 16:59:21, dnj wrote: > nit: capital "I" Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with nits https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/clien... File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/clien... cipd/client/cipd/client.go:1172: // Return exact same file as ReadSeekCloser. nit: this comment is no longer relevant https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/clien... cipd/client/cipd/client.go:1287: corrupt = local.IsCorruptionError(err) nit: you can deduplicate these checks by using named 'err' return value and checking it in the defer https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/inter... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/inter... cipd/client/cipd/internal/instancecache.go:96: if err = f.fs.EnsureFileGone(ctx, f.Name()); err != nil { nit: do not override error from a failed Close with success from EnsureFileGone. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/inter... cipd/client/cipd/internal/instancecache.go:103: func (f *cacheFile) UnderlyingFile() *os.File { why this is needed? If you sniff it through some interface, please document the intent there in this interface. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... File cipd/client/cipd/local/reader.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... cipd/client/cipd/local/reader.go:518: inst *packageInstance // the packageInstance this belongs to why this is needed here? https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... File cipd/client/cipd/local/reader_test.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... cipd/client/cipd/local/reader_test.go:52: type noopMarkCorrupt struct { the name looks weird now
https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/clien... File cipd/client/cipd/client.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/clien... cipd/client/cipd/client.go:1172: // Return exact same file as ReadSeekCloser. On 2017/05/22 20:41:39, Vadim Sh. wrote: > nit: this comment is no longer relevant Done. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/clien... cipd/client/cipd/client.go:1287: corrupt = local.IsCorruptionError(err) On 2017/05/22 20:41:38, Vadim Sh. wrote: > nit: you can deduplicate these checks by using named 'err' return value and > checking it in the defer Done. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/inter... File cipd/client/cipd/internal/instancecache.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/inter... cipd/client/cipd/internal/instancecache.go:96: if err = f.fs.EnsureFileGone(ctx, f.Name()); err != nil { On 2017/05/22 20:41:39, Vadim Sh. wrote: > nit: do not override error from a failed Close with success from EnsureFileGone. Done. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/inter... cipd/client/cipd/internal/instancecache.go:103: func (f *cacheFile) UnderlyingFile() *os.File { On 2017/05/22 20:41:39, Vadim Sh. wrote: > why this is needed? If you sniff it through some interface, please document the > intent there in this interface. Added comment; this is only used for tests (but unfortunately needs to be used in a different package). Is this enough? Probably the tests should be refactored, but that should probably be in another CL. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... File cipd/client/cipd/local/reader.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... cipd/client/cipd/local/reader.go:518: inst *packageInstance // the packageInstance this belongs to On 2017/05/22 20:41:39, Vadim Sh. wrote: > why this is needed here? oops! leftovers... nuked. https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... File cipd/client/cipd/local/reader_test.go (right): https://codereview.chromium.org/2872193003/diff/240001/cipd/client/cipd/local... cipd/client/cipd/local/reader_test.go:52: type noopMarkCorrupt struct { On 2017/05/22 20:41:39, Vadim Sh. wrote: > the name looks weird now Done.
The CQ bit was checked by iannucci@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by iannucci@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dnj@chromium.org Link to the patchset: https://codereview.chromium.org/2872193003/#ps260001 (title: "fix things")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 260001, "attempt_start_ts": 1495494722737810,
"parent_rev": "99f8e4b1c4b786ee903f40e5c71012bae7200cd1", "commit_rev":
"60ceda867cfa677fa85c4fe8517f6eb31d70d617"}
Message was sent while issue was closed.
Description was changed from ========== [cipd] delete bad cache files when failing to deploy them. This will let cipd naturally recover from a corrupted cipd cache without incurring the overhead of actually doing full cache verifications in the common case. R=vadimsh@chromium.org BUG= ========== to ========== [cipd] delete bad cache files when failing to deploy them. This will let cipd naturally recover from a corrupted cipd cache without incurring the overhead of actually doing full cache verifications in the common case. R=vadimsh@chromium.org BUG= Review-Url: https://codereview.chromium.org/2872193003 Committed: https://github.com/luci/luci-go/commit/60ceda867cfa677fa85c4fe8517f6eb31d70d617 ==========
Message was sent while issue was closed.
Committed patchset #14 (id:260001) as https://github.com/luci/luci-go/commit/60ceda867cfa677fa85c4fe8517f6eb31d70d617 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
