add boolean flag to run repo update in the background or not in Syncer.SyncRepo
Created by: mollylogue
Background behind this change: In working to test https://github.com/sourcegraph/sourcegraph/pull/28522 I realized that the asynchronous nature of syncer.SyncRepo
made testing unnecessarily difficult. It would be easier for this function to have synchronous behavior and then if we need the update to happen in the background, add the goroutine at a higher level for ease of testing these pieces. So, I'm proposing removing the goroutine from this function entirely. Below is my reasoning for why we can safely do that.
Currently, this goroutine is never being touched. The only place in the code that SyncRepo
is being called is the repo-lookup
endpoint in repo-updater
query to validate this
If we look where the RepoLookup
function in the repo updater client (which corresponds to this endpoint) is called, we see there are 3 cases
Cases:
-
repos.Add since this is for adding new repos,
repo
will always be nil andSyncRepo
will exit before the goroutine. -
set_user_public_repos.go but if you look on line 102, you see that if the repo already exists we return, so we again have a nil repo and exit before the goroutine in
SyncRepo
- dependency_indexing_scheduler.go This is the only one I'm not sure about, since we could potentially be updating existing repos. However, I still think that in this case the functionality should be synchronous as opposed to asynchronous but I'd like to validate that with someone who knows this part of the code already.
UPDATE: Adding in a boolean flag to do the sync (for non-nil repos) in the background or not and setting it to true
so this shouldn't change any functionality. Then when I call this function in https://github.com/sourcegraph/sourcegraph/pull/28522 I can set that boolean to false