-
Notifications
You must be signed in to change notification settings - Fork 140
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: support cloudflare options with setItem #255
Conversation
β Live Preview ready!
|
Codecov Report
@@ Coverage Diff @@
## main #255 +/- ##
==========================================
+ Coverage 77.96% 78.52% +0.56%
==========================================
Files 29 29
Lines 3417 3483 +66
Branches 521 522 +1
==========================================
+ Hits 2664 2735 +71
+ Misses 752 747 -5
Partials 1 1
π’ Have feedback on the report? Share it here. |
Refactor with namespace |
9cda23a
to
2b4bd93
Compare
Added a generic ttl parameters, as well as a default ttl configurable in the options. I noticed that you didn't nest the cloudflare r2 options under a key, so for now we need to be more explicit with any. This should be fixed in another PR. There's also a bunch of new bugs that this PR fixes in r2 and http, as we were using any before. @pi0 I think we need multiple PRs:
|
As i mentioned in other PR, it would make sense to support one kind of options interface (even if some option keys are not used by read operation). Particulary for
ππΌ |
If it is only for type, surely we can add |
65ebeac
to
ee82649
Compare
@pi0 I would be happy to revisit splitting TransactionOptions once it's actually needed. For now I've implemented everything in this PR according to your requirements. |
Thanks for the updates. As an update, i still need to double check all changes there are too many changes some are unrelated to the feature we want to add. |
@pi0 I had to change these things :
|
PR is stalled and with too many changes, have to rework this. (but again really appreciate your previous help on this β€οΈ ) |
π Linked issue
Resolves #32
First part of #236
β Type of change
π Description
This PR allows the ability to pass options to the setItem method while using the cloudflare driver :
cloudflare-kv-binding
andcloudflare-kv-http
driverssetItem
method.cloudflare-kv-http
to use the bulk API (to support all options).Options "API design"
For now I'm leverage the 3rd argument of the setItem method, but I was thinking that we could require users to pass them nested under a key (
{ cloudflare: {} }
), or use a 4th argument, in-case we want to keep the 3rd argument open for unstorage specific options... Thoughts ?π Checklist