Conversation
|
/run-security-scan |
alexcos20
left a comment
There was a problem hiding this comment.
AI automated code review (Gemini 3).
Overall risk: medium
Summary:
This pull request introduces the capability to send request bodies as a stream (P2PRequestBodyStream) through the P2P provider, aligning with the p2pStreamBody mechanism in ocean-node. This enables true streaming uploads, preventing the need to buffer entire request bodies in memory for large data transfers. The changes include adding new types and helper functions for stream handling, modifying dialAndStream and send methods to accept and process the request body stream, and updating the CI workflow to run on all branches.
Comments:
• [INFO][other] Changing branches to ** will run CI on all branches. This is fine for PRs, but be mindful of potential increased CI usage/costs if this also applies to pushes on many feature branches not intended for immediate merge.
• [INFO][other] The P2PRequestBodyStream type definition is clear and well-suited for handling various chunk types, ensuring flexibility for consumers of this API.
• [INFO][other] The toUint8ArrayChunk utility function correctly handles different input types (Uint8Array, string, ArrayBufferView) for conversion, making the stream processing robust.
• [INFO][other] The writeP2pRequestBodyLp function correctly implements the logic for streaming the body and terminating with an empty LP frame, which is crucial for interoperability with ocean-node's p2pStreamBody mechanism.
• [INFO][other] Adding p2pStreamBody: true to the initial payload when a request body stream is present is a good way to signal to the receiving ocean-node that a stream will follow, enabling proper parsing on their end. This ensures protocol compatibility.
• [ERROR][other] There are no tests included for this significant new streaming capability. Introducing streaming functionality, especially in a networking context, can have various edge cases (e.g., large files, slow connections, disconnections mid-stream).
Please add comprehensive unit and integration tests to cover:
- Sending various types of
P2PRequestBodyStream(e.g., string,Uint8Array,ArrayBufferView. - Sending an empty stream.
- Sending a very large stream (mocking or actual).
- Error handling during stream write.
- Ensure retries correctly re-send the stream argument (as it's just a reference, but needs to be clear it's not consumed on first attempt if that's the intention, or if it needs to be recreated).
• [WARNING][style] Consider adding JSDoc comments for the newrequestBodyparameter indialAndStreamandsendmethods to clearly explain its purpose and the types it accepts, improving API documentation for consumers.
* p2p impl * parallel http and p2p * rename job * fix field blob * serialize string * validate ddo * revert decryptor url * revert to http for now * fix signatures * lint fix * decrypt p2p and refactor * cleanup * cleanup and refactroing * p2p warmup * find peer * move functions to class level * remove unused checks * async node creation * reuse same provider * Updating ComputeExamples.md * simplift to hex and log err msg * lint fix * revert bad conversion * enable tcp and new method * fix methods mismatch * use signer in tests * Updating ComputeExamples.md * try peer id * use contracts version main * export contracts version main * get compute result, not only by url * lint fix * signature if no auth token * remove lint unused vars * encypt docker registry auth p2p * rename env to NODE * rename to NODE_ENDPOINT, NODE is reserved * Updating CodeExamples.md * Updating ComputeExamples.md * push,fetch config and no tls * Release 7.0.0-next.0 * bump uint8arrays to esm supported * getmultiaddr from peerid * remove mdns * print ci barge start script logs * remove explicit contracts version * start without typesense * Release 7.0.0-next.1 * improve check * circuit relay * circuit relay provider * libp2p circuit + evict bad connections * rename methods * fix unused auth in initializecompute * fix test init compute * Updating ComputeExamples.md * Release 7.0.0-next.2 * replace uint8arrays lib with native * fix exports name in pacakge json * Revert "fix exports name in pacakge json" This reverts commit 278ef0f. * Release 7.0.0-next.3 * devdep libp2p + fix exports * Release 7.0.0-next.4 * fix circuit parsing * Release 7.0.0-next.5 * keep only esential files * Release 7.0.0-next.6 * get node jobs and get node status * add fromtimestamp http * use direct command for status * Release 7.0.0-next.7 * allow multiaddr dial * Release 7.0.0-next.8 * fix review comments * Release 7.0.0-next.9 * p2p store discovered nodes & add dockerregistryauth * lint fix * Release 7.0.0-next.10 * export libp2p node * Release 7.0.0-next.11 * fetchnodelogs p2p method * Release 7.0.0-next.12 * create auth token with signed messge * Release 7.0.0-next.13 * ci use 127.0.0.1 * use older branch * Revert "ci use 127.0.0.1" This reverts commit db531ab. * add optional stream to command (#2065) * add optional stream to command * run CI on all branches, not just main * expose client protocols (#2066) * expose client protocols --------- Co-authored-by: GitHub Actions Bot <> Co-authored-by: alexcos20 <alex.coseru@gmail.com>
Changes proposed in this PR:
add optional stream to command
see https://github.com/oceanprotocol/ocean-node/pull/1318/changes#diff-30415eac87661d5c5d2e35045969efec41f8655a3e05ce130056c533ce77a902