feat/indexserver: extend UpdateIndexStatus request#1068
Conversation
Add repository state and failure message fields to UpdateIndexStatusRequest. This lets Sourcegraph depend on the expanded contract before Zoekt starts reporting failed indexing attempts. The zero enum value remains UNSPECIFIED so older clients stay backward compatible when they omit the new field.
keegancsmith
left a comment
There was a problem hiding this comment.
Is it not worth setting these fields already in this PR?
you are right. I updated the PR. Sourcegraph currently doesn't read the new fields so everything will work like before until I land the corresponding change in Sourcegraph. |
| failureMessage = indexErr.Error() | ||
| } else { | ||
| // We need to read from disk for IndexTime. | ||
| _, metadata, ok, err := c.findRepositoryMetadata(args) |
There was a problem hiding this comment.
I wonder if we should still do this anyways? If indexing fails we will still have the old shard on disk?
There was a problem hiding this comment.
We can do it best-effort. We might not have an index, for example, if indexing never worked or an index was deleted manually. Right now the idea is that we only update index time in Sourcegraph if status == success.
There was a problem hiding this comment.
Updated to read the metadata but not block on it.
This expands the
UpdateIndexStatusRPC contract so Sourcegraph can land its side of failed index status handling before Zoekt starts sending new runtime behavior. The request now carries an explicit repository state and optional failure message, while keepingUNSPECIFIEDas the zero value so older clients remain backward compatible when they omit the new field.This PR is intentionally contract-only. A follow-up Zoekt change will start reporting failed index attempts once the Sourcegraph side is ready to consume them.