Closed Bug 1319884 Opened 3 years ago Closed 3 years ago
Adapter feedback from kinto .js#589
:n1k0 in https://github.com/Kinto/kinto.js/pull/589 commented on a few changes I made to the FirefoxAdapter storage adapter for Kinto.js. (That PR was the one that allowed FirefoxAdapter to use an externally-managed Sqlite connection rather than creating its own; this was necessary to address feedback from bug 1253740.) :n1k0 observed that this caused two issues for FirefoxAdapter: - It now exports _init, which is simultaneously an implementation detail AND something that callers are supposed to know that they need to use. - FirefoxAdapter is now responsible for functioning whether or not a Sqlite connection is present. This adds a certain amount of complexity to the module. :n1k0 suggested I pick one or the other and convert existing uses. This bug represents the work that fixes these issues.
In doing this work, I looked at the other main client of kinto.js in Gecko is blocklist-clients.js, and it mainly calls collection.db.open() and collection.db.close(). In this approach, the Sqlite connection is internal to the Collection object. This does have a certain advantage in that 1. it's impossible to "forget" to _init the Sqlite connection (to create tables and so forth), 2. it makes the "common" case, where you don't care about the lifetime of the Sqlite connection, a little easier to manage. An ideal API would cover all use cases to exist. So far, I haven't been able to find such an API.
Comment on attachment 8813810 [details] Bug 1319884 - Address FirefoxAdapter feedback from kinto.js#589, https://reviewboard.mozilla.org/r/95170/#review96770
Attachment #8813810 - Flags: review?(mgoodwin) → review+
Comment on attachment 8813811 [details] Bug 1319884 - Leftover cleanups, https://reviewboard.mozilla.org/r/95172/#review96890
Attachment #8813811 - Flags: review?(MattN+bmo) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/836efdd6c83e Address FirefoxAdapter feedback from kinto.js#589, r=mgoodwin https://hg.mozilla.org/integration/autoland/rev/2648cedc9591 Leftover cleanups, r=MattN
You need to log in before you can comment on or make changes to this bug.