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

Preliminary cache migration code #674

Merged
merged 2 commits into from Sep 2, 2019

Conversation

@aphelionz
Copy link
Contributor

aphelionz commented Sep 1, 2019

This PR adds OrbitDb.attemptMigration

This handles the automatic migration the cache from multiple LevelDB stores (pre-0.21) to a single LevelDB store with multiple keys. (0.22+)

  • Tests for "default cache" scenario
  • Tests for passing the { directory } option into OrbitDB.create
@aphelionz aphelionz force-pushed the fix/cache-migration branch 2 times, most recently from 129f5de to 8eb40fc Sep 1, 2019
@aphelionz aphelionz referenced this pull request Sep 1, 2019
src/OrbitDB.js Outdated Show resolved Hide resolved
@haadcode

This comment has been minimized.

Copy link
Member

haadcode commented Sep 2, 2019

Looks good to me, generally. One little stylistic comment. I'm still wondering though, if it would be possible to move the cache migration to its own file, and isolate it completely out of the main class?

@aphelionz

This comment has been minimized.

Copy link
Contributor Author

aphelionz commented Sep 2, 2019

@haadcode Maybe we should start implementing a proper migrations folder. We could start with @shamb0t's 0.19-0.20, 0.20-0.21 migrations and include this as 0.21-0.22

@aphelionz

This comment has been minimized.

Copy link
Contributor Author

aphelionz commented Sep 2, 2019

And then simply have some call to this.migrate() at places in the OrbitDB class

@haadcode

This comment has been minimized.

Copy link
Member

haadcode commented Sep 2, 2019

@aphelionz that'd make sense 👍

@aphelionz

This comment has been minimized.

Copy link
Contributor Author

aphelionz commented Sep 2, 2019

@haadcode @shamb0t How's that look?

@haadcode

This comment has been minimized.

Copy link
Member

haadcode commented Sep 2, 2019

@haadcode @shamb0t How's that look?

LGTM 👍

cache loading test

this.attemptMigration

Migration data and cleanup

Linting

IPFS data

Revert "Linting"

This reverts commit e41bc4a.

Revert "IPFS data"

This reverts commit 299e0b7.

Better fixtures

package-lock.json

Test for directory options

directory option working

Fixing eventlog tests

Safer migration

Moving to migrations folder

Linting
@aphelionz aphelionz force-pushed the fix/cache-migration branch from 856c139 to e793edf Sep 2, 2019
@shamb0t

This comment has been minimized.

Copy link
Member

shamb0t commented Sep 2, 2019

Just a comment on styling but rest LGTM too 👍

src/migrations/0.21-0.22.js Outdated Show resolved Hide resolved
@aphelionz aphelionz merged commit 32d5dde into master Sep 2, 2019
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.