a8n: Introduce the ChangesetSyncer to sync changeset metadata in background
Created by: mrnugget
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 theSync
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 syncersSync
method. - Unit tests for the
ChangesetSyncer