Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | 2e2a637 | Commit Preview URL Branch Preview URL |
Feb 18 2026, 03:47 PM |
| override name: 'InvalidPieceCIDError' = 'InvalidPieceCIDError' | ||
|
|
||
| constructor(input: unknown, options?: SynapseErrorOptions) { | ||
| let msg = 'Invalid piece CID.' |
There was a problem hiding this comment.
| let msg = 'Invalid piece CID.' | |
| let msg = 'Invalid piece CID' |
| * @param signal - {@link AbortSignal} | ||
| * @returns The piece URL | ||
| */ | ||
| export async function pingProviders(providers: PDPProvider[], pieceCid: PieceCID, signal?: AbortSignal) { |
There was a problem hiding this comment.
This is the wrong name, it's not a ping and we shouldn't export it as a ping, "locate", "find", "confirm", "check" are all better words. findPieceOnProviders(), locatePieceFromProviders(), something like that?
| } | ||
| return false | ||
| }, | ||
| { concurrency: 5 } |
There was a problem hiding this comment.
Any reason not to increase this to 10 to make the most of concurrency potential? Most users will hopefully have 2 or 3 SPs they deal with so it won't matter much, but some will have more so we're doing a hunting expedition here for one that has the piece.
| * console.log(data) | ||
| * ``` | ||
| */ | ||
| export async function downloadAndValidate(options: downloadAndValidate.OptionsType): Promise<Uint8Array> { |
There was a problem hiding this comment.
I'd love to change this response type to a stream, but since we're starting from Promise<Uint8Array> in the sdk this is fine, but let's plan to make it the users's problem to deal with a stream in the future and we can ditch the accumulation in here
rvagg
left a comment
rvagg
left a comment
There was a problem hiding this comment.
Approving with some suggestions inline. The ping comment is more than a suggestion though, wrong word and I'd like to see that changed before merging.
3 participants
No description provided.