feat: create @helia/remote-pinner library #2
Conversation
|
Thank you for submitting this PR!
Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. |
|
ping @achingbrain @whizzzkid |
There was a problem hiding this comment.
this looks good as a wrapper class, but I had a few thoughts on the design:
- can this instead be a plugin?
- the plugin can subscribe to
AddPinEvents. - then run the pinning task on the remote service.
- This way we can have multiple pinning services instead of wrapping one inside the other?
src/index.ts
Outdated
| constructor (private readonly heliaInstance: Helia, private readonly remotePinningClient: RemotePinningServiceClient) { | ||
| } | ||
|
|
||
| private async getOrigins (otherOrigins: Pin['origins']): Promise<Set<string>> { |
There was a problem hiding this comment.
wondering if this can be static method.
There was a problem hiding this comment.
I think users could just use the pin response directly to get origins, they shouldn't be depending on this method
src/index.ts
Outdated
| } | ||
| } | ||
|
|
||
| export function createRemotePinner (heliaInstance: Helia, remotePinningClient: RemotePinningServiceClient, options?: HeliaRemotePinningOptions): HeliaRemotePinner { |
There was a problem hiding this comment.
would this interface not be like helia? also an opportunity to leverage mixins
There was a problem hiding this comment.
I'm not sure what you mean. This is a factory function and I think we want it to remain fairly lightweight. it's not doing much users couldn't do themselves, it's just a helper library
|
If it's desirable to use multiple pinning service endpoints, this module could take |
Co-authored-by: Alex Potsides <[email protected]>
Co-authored-by: Alex Potsides <[email protected]>
I think i'll leave this out for now as the error handling on this would be fairly excessive. We can add later if we want. created #5 to track |
addresses part of #1