fix: require passing repo options to migrator #30
Conversation
In order to successfully open a datastore we should accept config that tells us how to open the datastore. Refs: ipld/explore.ipld.io#65
|
The merge target for this is |
| const latestVersion = migrations.getLatestMigrationVersion() | ||
| const repoOptions = { | ||
| ... // the same storage backend/storage options passed to `ipfs-repo` | ||
| } |
There was a problem hiding this comment.
Is repoOptions still optional and we just need to make sure to pass it through if provided?
There was a problem hiding this comment.
It's been passed through by ipfs-repo since migrations were added.
There was a problem hiding this comment.
The readme api methods mention repoOptions as always optional, this is not the case as this change will throw errors. This doesn't matter for ipfs-repo since it is providing passing repoOptions to migrate and revert, but we should remove the optional jsdocs/readme as this is required.
There was a problem hiding this comment.
I've removed the optional notes in the readme. Which is a bit odd because it makes the options object mandatory, but I think that's ok for now as we are the only consumer of this module and we pass an options object. When I re-PR this against master I'll move the repo options out of the options object to make the options optional again.
src/commands.js
Outdated
| } | ||
|
|
||
| async function migrate ({ repoPath, migrations, to, dry, revertOk }) { | ||
| async function migrate ({ repoPath, repoOptions, migrations, to, dry, revertOk }) { |
There was a problem hiding this comment.
This file isn't used anymore since the cli.js is deleted, this can go away right?
There was a problem hiding this comment.
Good point, have removed.
jacobheun
left a comment
jacobheun
left a comment
There was a problem hiding this comment.
Changes look good.
For reference this is used by ipfs-repo@4.0.0 which is used in js-ipfs@0.48.x. (I looked this up to better understand the version migration paths since there are a couple major versions not yet being used)
|
Yep, I'm going to release this as v1.0.1 which should fix anyone using ipfs@0.48.1 or below who is building from source, then release ipfs@0.48.2 with this in the dist build for anyone not building from source, then port this to merge against v3.0.0, skipping v2.0.0 as ipfs@0.49.0 will ship with v3 if the uint8array migration stuff serendipitously passes an ipfs build, or v1 if it doesn't. |
Adds the changes released in `1.0.1` to master.
In order to successfully open a datastore we should accept config that tells us how to open the datastore.
Refs: ipld/explore.ipld.io#65