Palpatim/crash resuming from background #171
Conversation
…pping around AWSSQLLiteNormalizedCache
…configs - Renamed `AWSSQLLiteNormalizedCacheError` to `AWSAppSyncQueriesCacheError` - Deprecated `AWSSQLLiteNormalizedCache` - Deprecated `databaseURL` option to `AWSAppSyncClientConfiguration` - Deprecated `MutationCache` protocol
| // on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
| // express or implied. See the License for the specific language governing | ||
| // permissions and limitations under the License. | ||
| // |
There was a problem hiding this comment.
I think the license is ASL 1.0 and not apache. I may have missed this previously(If this isn't the first time adding this header)
There was a problem hiding this comment.
Thanks for pointing this out, will fix
| private let data = Expression<Data>("data") | ||
| private let recordState = Expression<String>("recordState") | ||
| private let timestamp = Expression<Date>("timestamp") | ||
| private let s3Bucket = Expression<String?>("s3Bucket") |
There was a problem hiding this comment.
With some customer asks about supporting more than 1 s3 file/ complex object per mutation, do you think it is a good idea to use this opportunity to change the schema to hold an array of s3 objects?
There was a problem hiding this comment.
I'd like to put that in a separate change, since this PR is a) targeted toward refactoring database handling; and b) quite large enough. :)
| import Foundation | ||
| import SQLite | ||
|
|
||
| final class SQLiteSerialization { |
| import Foundation | ||
|
|
||
| /// Errors thrown from the Queries Cache | ||
| public enum AWSAppSyncQueriesCacheError: Error { |
There was a problem hiding this comment.
Is there a way in which AWSAppSyncQueriesCacheError can inherit from AWSSQLLiteNormalizedCacheError so that the customer experience can be slightly better? We can still deprecate AWSSQLLiteNormalizedCacheError and get rid of it eventually.
There was a problem hiding this comment.
I considered some options but couldn't find a way to make this a non-breaking change. Value types like Enums can't inherit in Swift, and any change to the throwing type is going to potentially impact customers who are catching that specifically.
The three options I considered:
- Leave it as-is
- Remove
AWSSQLLiteNormalizedCacheErrorin favor of the new error type - Deprecate
AWSSQLLiteNormalizedCacheErrorin favor of the new error type
I don't favor Option 1. AWSSQLLiteNormalizedCacheError as a type name has some semantic confusion (What's a normalized cache? Why is SQLLite specified? Why is SQLite misspelled? :) )
I went back & forth between Option 2 & Option 3 and only settled on deprecation because that's our traditional way of doing things. I could be convinced to do Option 2--just removing it altogether. Deprecating means that customers who are actually taking meaningful action based on the error type will see a warning, but won't be forced to deal with the potential change in behavior. However, for customers who aren't doing anything critical (e.g. they're adding or suppressing logging based on the error type), it's an unexpected change to remove it.
A deprecation with a FixIt seemed the best compromise.
There was a problem hiding this comment.
Sounds good, thanks for the detailed explanation!
| /// - offlineMutations: The file path to create or connect to the cache for the offline mutation queue. | ||
| /// - queries: The file path to create or connect to the cache for the local query cache. | ||
| /// - subscriptionMetadataCache: The file path to create or connect to the cache for subscription metadata. | ||
| init(offlineMutations: URL?, queries: URL?, subscriptionMetadataCache: URL?) { |
There was a problem hiding this comment.
Should this be marked public?
There was a problem hiding this comment.
Great catch, thanks.
Issue #, if available:
#160
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.