Skip to content

a8n/enterprise: Introduce ChangesetSyncer

Warren Gifford requested to merge a8n/enterprise-changeset-sync into master

Created by: mrnugget

(Previous, non-enterprise PR: https://github.com/sourcegraph/sourcegraph/pull/5648)

This PR moves the syncing part of the CreateChangesets resolver method to a repos.ChangesetSyncer with a Sync method.

This Sync method is then called in the resolver itself and in the background, in the original Syncer.

The reason we're calling it in the Syncer is that we don't want to have another, separate loop that would send requests to GitHub, possibly triggering their API abuse detection mechanism.

The ChangesetSyncer.Sync method is integration tested through the GraphQL tests for CreateChangesets, which call the same method. That it's periodically called in the background I tested manually with multiple changesets.

Do we need a feature flag? Not sure. Right now the changeset syncer does nothing if there are no changesets in the database.

What's missing? Depends on the level of polish we're going for. Some ideas:

  • Maybe aeparate feature-flag-ENV-var. Currently the syncer calls the ChangesetSyncer on every run. If there are no changesets in the database (99% of our customers), then it does nothing. I'm not sure whether we need a flag in addition to that.
  • Optimization: we're currently writing to the database with ever sync, even if nothing has changed. I think that is fine for now, but should be on top of the list regarding "improve syncing process"
  • The changeset sync currently gets triggered when we receive a trigger-sync-signal. That happens when a user saves an external service, for example. In that case, we probably don't want to sync changesets. But maybe we do want a "hard refresh" button for changesets somewhere in the UI?
  • Refactor code so that the changeset syncer uses the repos.Store from the Syncer instead of constructing its own in the Sync method
  • Maybe introduce a higher-level syncer that gets initialized with a ChangesetSyncer and a ReposSyncer. In its Run method this new syncer calls both syncers Sync method.
  • Unit tests for the ChangesetSyncer

Merge request reports