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.

Sign up for GitHub

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

Jump to bottom

Drop support of key & value types other than string and Buffer #179

Merged
merged 1 commit into from
Sep 15, 2019
Merged

Drop support of key & value types other than string and Buffer #179

merged 1 commit into from
Sep 15, 2019

Conversation

vweevers
Copy link
Member

@vweevers vweevers commented Sep 8, 2019

Closes #174. WIP.

@vweevers vweevers added the semver-major Changes that break backward compatibility label Sep 8, 2019
@vweevers vweevers added this to In progress in Level (old board) via automation Sep 8, 2019
vweevers
vweevers commented Sep 8, 2019
View reviewed changes
index.js Show resolved Hide resolved
vweevers
vweevers commented Sep 13, 2019
View reviewed changes
index.js Outdated Show resolved Hide resolved
vweevers
vweevers commented Sep 14, 2019
View reviewed changes
UPGRADING.md
if (err) throw err
})
})
```
Copy link
Member Author

@vweevers vweevers Sep 14, 2019 •
edited

Choose a reason for hiding this comment

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

Should we include this? I wrote it to see if it could work, and it does for common cases, but not if:

  • You target old browsers that don't support binary keys (IE, Edge and a few others, the ones in red here)
  • You used custom key or value types (numbers etc); these come out stringified
  • You mixed key types (e.g. strings and buffers); keys may conflict.

We could (also) choose to:

  • Drop browsers that don't support binary keys
  • Store keys as base64 strings for these browsers (you won't be able to read existing string data though).
  • Use a new database prefix (to avoid reading old data altogether)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, lets include this.

@vweevers vweevers marked this pull request as ready for review September 14, 2019 06:52
vweevers
vweevers commented Sep 14, 2019
View reviewed changes
UPGRADING.md
if (err) throw err
})
})
```
Copy link
Member Author

Choose a reason for hiding this comment

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

I also considered adding the upgrade method to the manifest, which level (and layers in between) could detect and then proxy, but it's too early for that. We should first add manifests to abstract-leveldown and work out the details there.

@vweevers vweevers moved this from In progress to Review in Level (old board) Sep 14, 2019
@vweevers vweevers mentioned this pull request Sep 14, 2019
Closed
31 tasks
@vweevers vweevers merged commit f207ae6 into master Sep 15, 2019
Level (old board) automation moved this from Review to Done Sep 15, 2019
@vweevers vweevers deleted the drop-custom-types branch September 15, 2019 18:52
This was referenced Sep 18, 2019
Closed
Merged
achingbrain added a commit to ipfs/js-ipfs-repo-migrations that referenced this pull request Jan 28, 2021
@achingbrain
feat: migration 10 to allow upgrading level in the browser
cb7167c 
We use the [level](https://www.npmjs.com/package/level) module to supply
either [leveldown](http://npmjs.com/package/leveldown) or [level-js](https://www.npmjs.com/package/level-js)
to [datastore-level](https://www.npmjs.com/package/datastore-level) depending
on if we're running under node or in the browser.

`level@6.x.x` upgrades the `level-js` dependency from `4.x.x` to `5.x.x`
which includes the changes from [Level/level-js#179](Level/level-js#179)
so `>5.x.x` requires all database keys/values to be Uint8Arrays and they
can no longer be strings.

We already store values as Uint8Arrays but our keys are strings, so here
we add a migration to converts all datastore keys to Uint8Arrays.

N.b. `leveldown` already does this conversion for us so this migration
only needs to run in the browser.
@achingbrain achingbrain mentioned this pull request Jan 28, 2021
Merged
achingbrain added a commit to ipfs/js-ipfs-repo-migrations that referenced this pull request Jan 28, 2021
@achingbrain
feat: migration 10 to allow upgrading level in the browser
5a6a782 
We use the [level](https://www.npmjs.com/package/level) module to supply
either [leveldown](http://npmjs.com/package/leveldown) or
[level-js](https://www.npmjs.com/package/level-js)
to [datastore-level](https://www.npmjs.com/package/datastore-level) depending
on if we're running under node or in the browser.

`level@6.x.x` upgrades the `level-js` dependency from `4.x.x` to `5.x.x`
which includes the changes from
[Level/level-js#179](Level/level-js#179)
so `>5.x.x` requires all database keys/values to be Uint8Arrays and they
can no longer be strings.

We already store values as Uint8Arrays but our keys are strings, so here
we add a migration to converts all datastore keys to Uint8Arrays.

N.b. `leveldown` already does this conversion for us so this migration
only needs to run in the browser.
achingbrain added a commit to ipfs/js-ipfs-repo-migrations that referenced this pull request Jan 28, 2021
@achingbrain
feat: migration 10 to allow upgrading level in the browser
b66c3a4 
We use the [level](https://www.npmjs.com/package/level) module to supply
either [leveldown](http://npmjs.com/package/leveldown) or
[level-js](https://www.npmjs.com/package/level-js)
to [datastore-level](https://www.npmjs.com/package/datastore-level) depending
on if we're running under node or in the browser.

`level@6.x.x` upgrades the `level-js` dependency from `4.x.x` to `5.x.x`
which includes the changes from
[Level/level-js#179](Level/level-js#179)
so `>5.x.x` requires all database keys/values to be Uint8Arrays and they
can no longer be strings.

We already store values as Uint8Arrays but our keys are strings, so here
we add a migration to converts all datastore keys to Uint8Arrays.

N.b. `leveldown` already does this conversion for us so this migration
only needs to run in the browser.
achingbrain added a commit to ipfs/js-ipfs-repo-migrations that referenced this pull request Jan 28, 2021
@achingbrain
feat: migration 10 to allow upgrading level in the browser
3f43681 
We use the [level](https://www.npmjs.com/package/level) module to supply
either [leveldown](http://npmjs.com/package/leveldown) or
[level-js](https://www.npmjs.com/package/level-js)
to [datastore-level](https://www.npmjs.com/package/datastore-level) depending
on if we're running under node or in the browser.

`level@6.x.x` upgrades the `level-js` dependency from `4.x.x` to `5.x.x`
which includes the changes from
[Level/level-js#179](Level/level-js#179)
so `>5.x.x` requires all database keys/values to be Uint8Arrays and they
can no longer be strings.

We already store values as Uint8Arrays but our keys are strings, so here
we add a migration to converts all datastore keys to Uint8Arrays.

N.b. `leveldown` already does this conversion for us so this migration
only needs to run in the browser.
achingbrain added a commit to ipfs/js-ipfs-repo-migrations that referenced this pull request Jan 29, 2021
@achingbrain
feat: migration 10 to allow upgrading level in the browser (#59)
7dc562b 
We use the [level](https://www.npmjs.com/package/level) module to supply either [leveldown](http://npmjs.com/package/leveldown) or [level-js](https://www.npmjs.com/package/level-js) to [datastore-level](https://www.npmjs.com/package/datastore-level) depending on if we're running under node or in the browser.

`level@6.x.x` upgrades the `level-js` dependency from `4.x.x` to `5.x.x` which includes the changes from [Level/level-js#179](Level/level-js#179) so `>5.x.x` requires all database keys/values to be Uint8Arrays and they can no longer be strings.

We already store values as Uint8Arrays but our keys are strings, so here we add a migration to converts all datastore keys to Uint8Arrays.

N.b. `leveldown` already does this conversion for us so this migration only needs to run in the browser.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ralphtheninja ralphtheninja ralphtheninja approved these changes

Assignees
No one assigned
Labels
semver-major Changes that break backward compatibility
Projects
No open projects
Level (old board)
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drop support of key & value types other than string and Buffer
2 participants
@vweevers @ralphtheninja