chore: Add ability to plumb opaque ID into a shard at index time with zoekt-git-index
#6
Conversation
|
/review |
![sourcebot-review-agent[bot]](https://avatars.githubusercontent.com/u/135396723?s=60&v=4){kind=small}
| deltaShardNumberFallbackThreshold := flag.Uint64("delta_threshold", 0, "upper limit on the number of preexisting shards that can exist before attempting a delta build (0 to disable fallback behavior)") | ||
| languageMap := flag.String("language_map", "", "a mapping between a language and its ctags processor (a:0,b:3).") | ||
| tenantID := flag.Int("tenant_id", 0, "tenant ID to use for indexed repositories") | ||
| repoID := flag.Uint("repo_id", 0, "opaque ID to use for indexed repositories. Surfaces as `RepositoryID` in the REST search response.") |
There was a problem hiding this comment.
The new flag repoID is added but this flag's value is never used or passed along to indexing options. You should integrate repoID into the relevant indexing option struct, for example, setting it on opts or gitOpts.RepositoryDescription to ensure it takes effect during indexing.
| opts := cmd.OptionsFromFlags() | ||
| opts.IsDelta = *isDelta | ||
| opts.RepositoryDescription.TenantID = *tenantID | ||
| opts.RepositoryDescription.ID = uint32(*repoID) |
There was a problem hiding this comment.
The new assignment opts.RepositoryDescription.ID = uint32(*repoID) references *repoID which is not declared or initialized anywhere in the provided context. Please ensure that repoID is declared as a flag or variable and properly parsed before usage.
| idString := sec.Options.Get("repoid") | ||
| if idString != "" { | ||
| id, _ := strconv.ParseUint(idString, 10, 32) | ||
| desc.ID = uint32(id) | ||
| } |
There was a problem hiding this comment.
The error from strconv.ParseUint is ignored; this could cause silent failures if 'repoid' is malformed. Consider returning or logging the error to handle invalid input gracefully.
| idString := sec.Options.Get("repoid") | ||
| if idString != "" { | ||
| id, _ := strconv.ParseUint(idString, 10, 32) | ||
| desc.ID = uint32(id) | ||
| } |
There was a problem hiding this comment.
sec.Options is a collection of all zoekt prefixed git configs that are set in repo compile utils. E.g.,:
Before this change, if zoekt.repoid is not set, id will resolve to 0 and override anything that is already set in desc.ID. We could use zoekt.repoid to set the id instead of plumbing it like we are here, but this was easier since we don't know the repo's id when we construct the zoekt.* payload (in gitConfig)
2 participants
This PR adds the
repo_idoption tozoekt-git-index, allowing us to plumb a repository's database ID (Sourcebot ID) into it's corresponding shard(s) at index time. When an ID for a repository is present, it will surface as theRepositoryIDproperty for each file match (see here). This is useful for correlating additional metadata from the db.