feat(bindings/nodejs): Add ReadOptions and ReaderOptions support for new options API#6312
Conversation
dosubot
bot
added
size:XXL
kingsword09
force-pushed
the
feat-nodejs-read-opts
branch
from
6ce3d5e to
cb41253
Compare
kingsword09
force-pushed
the
feat-nodejs-read-opts
branch
from
cb41253 to
b456cac
Compare
| describe.runIf(capability.read && capability.write)('async read options', () => { | ||
| test('read with range', async () => { | ||
| const size = 5 * 1024 * 1024 | ||
| const size = 3 * 1024 * 1024 |
There was a problem hiding this comment.
Limit range request to 3MB because the etcd service enforces a 4MB gRPC message size limit, and larger requests will fail.
|
@Xuanwo @suyanhanx We need to clarify whether the “size” parameter should represent the number of bytes to read after the offset (length-based), or if it should specify the absolute end bound (end-based) for the read operation. I noticed that in the Python binding, “size” is actually used as the end bound, but the name “size” doesn’t seem to accurately reflect this usage. Currently, I am using “size” to indicate the number of bytes to read after the offset. Should I align this behavior with the Python binding, where “size” is used as the end bound? |
Hi, I believe your understanding is correct. The size refers to this reading's size, and our actual read range should be |
|
Hi, @kingsword09, is this PR ready to go? |
Yes |
2 participants
Which issue does this PR close?
Related to #6281.
Rationale for this change
This PR adds support for opendal::options::ReadOptions and opendal::options::ReaderOptions conversion in the nodejs bindings. This is part of the migration to the new options API outlined in RFC-6213 (#6213).
What changes are included in this PR?
Are there any user-facing changes?
Yes, users can now add options to their read and reader requests