Skip to content
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

Fix/store management #74

Merged
merged 1 commit into from Sep 23, 2019
Merged

Fix/store management #74

merged 1 commit into from Sep 23, 2019

Conversation

@aphelionz
Copy link
Member

@aphelionz aphelionz commented Sep 16, 2019

Three primary changes here.

  1. Adding an onDrop hook for orbit-db to do necessary cache management.
  2. Adding an onLoad hook for orbit-db to do necessary cache management.
  3. Wrapping Store.load in a try/catch so that if the Store is closed during an async load, it will catch the "Database is not open" error and instead display a warning.
src/Store.js Outdated Show resolved Hide resolved
src/Store.js Outdated Show resolved Hide resolved
@shamb0t
Copy link
Member

@shamb0t shamb0t commented Sep 18, 2019

Only concern here is load may just fail with a console.warn and not emit the ready event (which it shouldn't in that case), but are the only cases this will happen that await isnt used or the cache is closed?

@haadcode
Copy link
Member

@haadcode haadcode commented Sep 18, 2019

Quick one: dist/orbit-db-store.min.js.map should not be committed to the repo (any repo)

@aphelionz aphelionz closed this Sep 19, 2019
@aphelionz aphelionz reopened this Sep 19, 2019
src/Store.js Outdated Show resolved Hide resolved
@@ -163,7 +163,7 @@ class Store {

async close () {
if (this.options.onClose) {
await this.options.onClose(this.address.toString())
await this.options.onClose(this)
Copy link
Member

@shamb0t shamb0t Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not send just the address?

Copy link
Member Author

@aphelionz aphelionz Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other on____ functions require this so we can easily do db._cache.open in orbit-db I changed it just to be consistent. I don't think onClose is currently used anywhere else.

Copy link
Member

@shamb0t shamb0t Sep 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 what do you think of sending just the address in the other functions as well and retrieving the store the same way we do in onClose?

Copy link
Member Author

@aphelionz aphelionz Sep 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it doesn't make too much of a difference since we can get what we need from OrbitDB.stores - should be equivalent objects. Just a bit more code on the OrbitDB side of things.

…catch

Build

v2.7.0-rc5

opening on load

removing map file

ignoring map files

fixing package.json

removing band-aids

try / catch around drop and restoration of cache.open in load

onClose and onDrop updates

onLoad hook

remove default directory and try/catch
@aphelionz aphelionz force-pushed the fix/store-management branch from 57bccc4 to f65de73 Sep 23, 2019
@aphelionz aphelionz merged commit 1c70f92 into master Sep 23, 2019
4 checks passed
@aphelionz aphelionz deleted the fix/store-management branch Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants