-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Using config to decide base query or delta query #386
Conversation
@@ -54,7 +54,19 @@ final class InitialSyncOperation: AsynchronousOperation { | |||
} | |||
|
|||
let lastSyncMetadata = getLastSyncMetadata() | |||
lastSyncTime = lastSyncMetadata?.lastSync | |||
if let lastSync = lastSyncMetadata?.lastSync { | |||
let lastSyncDate = Date(timeIntervalSince1970: TimeInterval(lastSync) / 1_000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be convenient to extend @drochetti's TimeInterval
extension that provides unit-based constructors with a milliseconds
so you could just do lastSyncDate = Date(timeIntervalSince1970: TimeInterval.milliseconds(lastSync))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm yes, this would be cool. This is not currently in master
, so adding a todo, and
if let lastSync = lastSyncMetadata?.lastSync { | ||
let lastSyncDate = Date(timeIntervalSince1970: TimeInterval(lastSync) / 1_000) | ||
let secondsSinceLastSync = (lastSyncDate.timeIntervalSinceNow * -1) | ||
if secondsSinceLastSync < 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just if lastSyncDate > Date() { ... assume base query ... }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked at doing this, but I need secondsSinceLastSync
a couple lines down to determine if we should do a delta query.
AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/InitialSync/InitialSyncOperation.swift
Outdated
Show resolved
Hide resolved
AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/InitialSync/InitialSyncOperation.swift
Outdated
Show resolved
Hide resolved
bf03452
to
02a09e7
Compare
60f222b
to
9111186
Compare
02a09e7
to
ea5f132
Compare
AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/InitialSync/InitialSyncOperation.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider a backlog item for the TODO, otherwise LGTM
ea5f132
to
52bd777
Compare
52bd777
to
058da3e
Compare
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.