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

feat: experimental operation batching #240

Merged
merged 16 commits into from
Jul 4, 2023
Merged

Conversation

Hebilicious
Copy link
Member

@Hebilicious Hebilicious commented May 31, 2023

Resolves #198

Description

Add the following APIs :

  • setItems()
  • getItems()

The getItems() API supports 2 syntaxes for setting metadata on multiple keys.
In future PRs we can individually implement more efficient batching for drivers that have a true batching API. There's the localstorage driver that has a getItems implemented has a proof-of-concept, but it can be removed as it's doing the same as the fallback.

@pi0 Note that the getItems method will retrieve the first mountpoint. Something we could do is to use Promise.any on all the mountpoints to get the first one that succeed. I'm not sure if that's desirable as it will make un-needed reads... But maybe that's what is expected with multiple mountpoints?

TODO :

  • Base implementation
  • Tests
  • Docs

src/drivers/localstorage.ts Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
@pi0 pi0 self-assigned this Jul 4, 2023
@pi0
Copy link
Member

pi0 commented Jul 4, 2023

Update: Just pushed a refactor with a shared internal runBatch utility that is able to run multi level parallel (per-driver batching support when possible 🚀 ) in addition to some minor refactors.

@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #240 (26b0f6d) into main (8353c6d) will increase coverage by 0.25%.
The diff coverage is 80.00%.

❗ Current head 26b0f6d differs from pull request most recent head 9cafabb. Consider uploading reports for the commit 9cafabb to get more accurate results

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
+ Coverage   75.99%   76.24%   +0.25%     
==========================================
  Files          25       25              
  Lines        2895     3027     +132     
  Branches      417      441      +24     
==========================================
+ Hits         2200     2308     +108     
- Misses        694      718      +24     
  Partials        1        1              
Impacted Files Coverage Δ
src/storage.ts 85.30% <76.10%> (-2.79%) ⬇️
src/drivers/http.ts 85.56% <100.00%> (ø)
src/types.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@pi0
Copy link
Member

pi0 commented Jul 4, 2023

@Hebilicious Something regarding unstorage API design:

User => (not normalized input) => Storage => (normalized input) => Driver => (not normalized output) => Storage => (normalized output)

This way we make sure both mistakes and not normalized (non promises, bad keys, etc) by Users and Driver authors are gracefully handled by Storage layer.

@nuxt-studio
Copy link

nuxt-studio bot commented Jul 4, 2023

Live Preview ready!

Name Edit Preview Latest Commit
unstorage Edit on Studio ↗︎ View Live Preview 9cafabb

@pi0 pi0 merged commit cb8577b into unjs:main Jul 4, 2023
so1ve pushed a commit to so1ve/unstorage that referenced this pull request Jul 8, 2023
* fix(prefixStorage): prefix `getItemRaw` and `setItemRaw` (unjs#232)

* fix(github): fetchFiles should return files (unjs#229)

* chore(deps): update all non-major dependencies (unjs#220)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore: update eslint

* test: skip cloudflare-kv-http on node >= 18

* chore(release): v1.6.1

* docs: add social share image

* chore: update deps

* docs: fix typo (unjs#239)

Change `environemnt` to `environment`
Update cloudflare-kv-http.md

* chore: update dependencies

* feat: generic type support (unjs#237)

* refactor: fix issues with typescript strict (unjs#250)

* chore: add type check to ci

* ci: skip flaky azure tests

* chore(release): v1.7.0

* chore(deps): update all non-major dependencies

* docs: fix typo (unjs#252)

* chore(deps): update all non-major dependencies

* test: add test for `github` driver (unjs#259)

* feat: experimental operation batching (unjs#240)

Co-authored-by: Pooya Parsa <pooya@pi0.io>

* feat(cloudflare-kv): support `base` option for keys (unjs#261)

* feat: `cloudflare-r2-binding` driver (unjs#235)

* fix: add missing `cloudflareR2Binding` to the `builtinDrivers`

* chore: update dev dependencies

* chore(release): v1.8.0

* Fix typescript checks

* add typehint

* Install execa

* Write test code

* Remove not using import

---------

Co-authored-by: 魔王少年 <q267009886.work@gmail.com>
Co-authored-by: Andrei Dyldin <and@cesbo.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Pooya Parsa <pooya@pi0.io>
Co-authored-by: Sébastien Chopin <seb@nuxt.com>
Co-authored-by: Neelansh Mathur <53081208+neelansh15@users.noreply.github.com>
Co-authored-by: 魔王少年 <q267009886.tw@gmail.com>
Co-authored-by: Alex Duval <alexduval71@gmail.com>
Co-authored-by: Hebilicious <xsh4k3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support setItems and getItems for operation batching
2 participants