Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

insights: don't error on empty repo in commit indexer#38091

Merged
leonore merged 11 commits into
mainfrom
insights/empty-repo-commit-error
Jul 4, 2022
Merged

insights: don't error on empty repo in commit indexer#38091
leonore merged 11 commits into
mainfrom
insights/empty-repo-commit-error

Conversation

@leonore

@leonore leonore commented Jul 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

closes #37151

instead of erroring we mark the repo as successfully last indexed in commit_index_metadata

Test plan

Added unit tests for the new helper and for the function.

Tested on enterprise-codeinsights (before/after):
Screenshot 2022-07-01 at 13 46 51
image

and we successfully insert the metadata.
image

@leonore leonore requested a review from a team July 1, 2022 12:49
@cla-bot cla-bot Bot added the cla-signed label Jul 1, 2022
@plz-review

plz-review Bot commented Jul 1, 2022

Copy link
Copy Markdown

Plz Review!
Review this PR using plz

@sourcegraph-bot

sourcegraph-bot commented Jul 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 4d85487...ab78396.

Notify File(s)
@sourcegraph/code-insights-backend enterprise/internal/insights/compression/worker.go
enterprise/internal/insights/compression/worker_test.go

chwarwick
chwarwick approved these changes Jul 1, 2022
View reviewed changes

@chwarwick chwarwick left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! it's unfortunate we don't get better error messages back

@leonore

leonore commented Jul 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Nice! it's unfortunate we don't get better error messages back

@charwick I was just about to ping you to say we could return nil a level above in the getCommits function if we see the empty error message (i.e. not assert on errors.Is later), I think that would remove the error log. or we could make it warning. it's done through the observation pkg which I'm not so familiar with

edit: something like

[         worker] t=2022-07-01T13:11:15+0000 lvl=warn msg=CommitIndexer.GetCommits 
repo name=github.com/leonore/empty-repo error="empty repo - metadata updated succesfully"

chwarwick
chwarwick reviewed Jul 1, 2022
View reviewed changes
Comment thread enterprise/internal/insights/compression/worker.go Outdated
@leonore leonore merged commit d0150b3 into main Jul 4, 2022
@leonore leonore deleted the insights/empty-repo-commit-error branch July 4, 2022 08:49
vovakulikov pushed a commit that referenced this pull request Jul 20, 2022
@vovakulikov
insights: don't error on empty repo in commit indexer (#38091)
a2cf2b0
@leonore leonore mentioned this pull request Nov 17, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@chwarwick chwarwick chwarwick approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

insights: Commit Indexer returns errors on empty repos

3 participants

@leonore @sourcegraph-bot @chwarwick