Skip to content

chore: Add ability to plumb opaque ID into a shard at index time with zoekt-git-index #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2025
Merged

chore: Add ability to plumb opaque ID into a shard at index time with zoekt-git-index #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/zoekt-git-index/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func run() int {
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.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


cpuProfile := flag.String("cpuprofile", "", "write cpu profile to `file`")

Expand Down Expand Up @@ -77,6 +78,7 @@ func run() int {
opts := cmd.OptionsFromFlags()
opts.IsDelta = *isDelta
opts.RepositoryDescription.TenantID = *tenantID
opts.RepositoryDescription.ID = uint32(*repoID)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


var branches []string
if *branchesStr != "" {
Expand Down
7 changes: 5 additions & 2 deletions gitindex/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,11 @@ func setTemplatesFromConfig(desc *zoekt.Repository, repoDir string) error {
}
}

id, _ := strconv.ParseUint(sec.Options.Get("repoid"), 10, 32)
desc.ID = uint32(id)
idString := sec.Options.Get("repoid")
if idString != "" {
id, _ := strconv.ParseUint(idString, 10, 32)
desc.ID = uint32(id)
}
Comment on lines +254 to +258
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +254 to +258
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed?

Copy link
Author

@brendan-kellam brendan-kellam May 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sec.Options is a collection of all zoekt prefixed git configs that are set in repo compile utils. E.g.,:

image

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)


if desc.RawConfig == nil {
desc.RawConfig = map[string]string{}
Expand Down
Loading