ARC-814 RepoSyncState table as source of truth #928
Conversation
| @@ -104,7 +112,7 @@ export default async ( | |||
| ) | |||
| ); | |||
|
|
|||
| const failedConnections = getFailedConnections( | |||
| const failedConnections = await getFailedConnections( | |||
There was a problem hiding this comment.
does this mean failedConnections has always been an unresolved promise?
There was a problem hiding this comment.
No, it used to be a sync function which I changed to async for the booleanFlag call
| if (repo) { | ||
| this.repoUpdatedAt = new Date(repo.repository?.updated_at || Date.now()); | ||
| this.repoUpdatedAt = new Date(repo.repository?.updated_at ?? Date.now()); |
There was a problem hiding this comment.
Why use ?? instead of || here?
There was a problem hiding this comment.
?? is null coalescing since we're looking for if it's undefined or null. A test that we use is using new Date(0) because we wanted to compare the specific 8601 date and instead of using a random number, 0 seemed easy, but 0 is falsy, hence it would return Date.now() and fail the test.
|
Please link the filled up FD ticket |
|
|
||
| function mapSyncStatus(syncStatus: string): string { | ||
| function mapSyncStatus(syncStatus: SyncStatus = SyncStatus.PENDING): string { |
There was a problem hiding this comment.
In which case this method will have to use the default parameter?
There was a problem hiding this comment.
Subscription.syncStatus is optional, we needed a way to handle in the off chance that it wasn't set yet, but almost certainly would be by this point as this is for current subscriptions which any new subscriptions is automatically synced and set to PENDING. https://github.com/atlassian/github-for-jira/blob/main/src/models/subscription.ts#L166
| @@ -94,7 +95,7 @@ export default class Installation extends Sequelize.Model { | |||
| await this.destroy(); | |||
| } | |||
|
|
|||
| async subscriptions(): Promise<Subscription[]> { | |||
| async subscriptions(): Promise<SubscriptionClass[]> { | |||
There was a problem hiding this comment.
I wonder what's the difference between Subscription and SubscriptionClass? And how does this type change affect things?
There was a problem hiding this comment.
yeah, we need to fix that at some point. It's because we're using an export default class called Subscription at src/models/subscription but we also have a export const value called Subscription which is an instance of the class that's being built by Sequelize and provided by src/models/index. Values and Class types are different and you cannot place a value as a type of a function. If you look at the imports at the top of the file, I had to import both the class type and the value as you need to refer to the value if you want to do operations on the DB with sequelize or else it'll just error at runtime.
Just a few things:
|
|
@KAbakumovAtl done. |
Adding new FF and making all 'reads' for repo sync state come from the new table. We're still writing to the JSON blob column so we can switch back in case of issues. Next step after fully deployed is to remove the repoSyncState column and all references to it.