-
Notifications
You must be signed in to change notification settings - Fork 4
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: add support for libp2p ContentRouting and PeerRouting #44
Conversation
Enables passing a client instance as a libp2p service which will detect it's `ContentRouting` and `PeerRouting` capabilities and configure them for use.
Conceptually makes sense to me. Thanks! Maybe add a note about this in https://github.com/ipfs/helia-delegated-routing-v1-http-api/blob/main/packages/client/README.md or https://github.com/ipfs/helia-delegated-routing-v1-http-api/blob/main/README.md since this presumably be one of the main users of the client? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Just some comments around improvements for testing and documentation that could be made.
export function createDelegatedRoutingV1HttpApiClient (url: URL | string, init: DelegatedRoutingV1HttpApiClientInit = {}): DelegatedRoutingV1HttpApiClient { | ||
return new DefaultDelegatedRoutingV1HttpApiClient(new URL(url), init) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for supporting strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general if we can push validation of an argument type out of a consumer it's a good thing. Otherwise this can snowball quickly and we have to guess what the contents of a string are, usually at several points in a module if we have multiple entry points.
We suffered from this previously with "what's a CID?" is it "QmFoo", is it "/ipfs/QMFoo", etc. Pushing all that into the CID
class vastly simplified the codebase.
Of course for a user it's nicer to be able to pass 'http://example.org'
instead of new URL('http://example.org')
so it's probably worth the tradeoff for the create...
factory functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be a straightforward one where it will fail quickly if an invalid string is passed.
throw new Error('PeerRouting not found') | ||
} | ||
|
||
await drain(routing.getClosestPeers(Uint8Array.from([0, 1, 2, 3, 4]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the assertion here is not obvious. it seems like it should be either
await drain(routing.getClosestPeers(Uint8Array.from([0, 1, 2, 3, 4]))) | |
expect(async () => await drain(routing.getClosestPeers(Uint8Array.from([0, 1, 2, 3, 4])))).toThrow() | |
// or | |
expect(async () => await drain(routing.getClosestPeers(Uint8Array.from([0, 1, 2, 3, 4])))).to.haveLength(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added an assertion.
Note that you have to use the eventually
keyword to make the kind of assertions you're trying to make above. See the chai-as-promised docs.
drain also has a return type of void
(as it just empties the iterator) - to assert contents use all or just length if keeping all iterator contents around might make the process run out of memory:
// will not work because it'll try to assert that the promise (not the promised value)
// returned from the async function has a 'length' property of 0
expect(async () => await drain(routing.getClosestPeers(...))).to.haveLength(0)
// instead:
await expect(all(routing.getClosestPeers(...))).to.eventually.have.lengthOf(0)
// or:
await expect(all(routing.getClosestPeers(...))).to.eventually.be.empty()
// instead:
await expect(length(routing.getClosestPeers(...))).to.eventually.equal(0)
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks
export function createDelegatedRoutingV1HttpApiClient (url: URL | string, init: DelegatedRoutingV1HttpApiClientInit = {}): DelegatedRoutingV1HttpApiClient { | ||
return new DefaultDelegatedRoutingV1HttpApiClient(new URL(url), init) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be a straightforward one where it will fail quickly if an invalid string is passed.
## [@helia/delegated-routing-v1-http-api-client-v1.1.0](https://github.com/ipfs/helia-delegated-routing-v1-http-api/compare/@helia/delegated-routing-v1-http-api-client-v1.0.1...@helia/delegated-routing-v1-http-api-client-v1.1.0) (2023-10-30) ### Features * add support for libp2p ContentRouting and PeerRouting ([#44](#44)) ([ddfff1b](ddfff1b)) ### Documentation * update project config and readmes ([#47](#47)) ([31eb6f7](31eb6f7))
## 1.0.0 (2024-06-20) ### ⚠ BREAKING CHANGES * returns IPNS objects are from ipns@9.x.x * bump multiformats from 12.1.3 to 13.0.0 (#75) * method signatures have changed to be closer to the delegated http routing v1 API spec ### Features * add support for libp2p ContentRouting and PeerRouting ([#44](#44)) ([ddfff1b](ddfff1b)) * allow skipping IPNS record validation ([#101](#101)) ([ec0ba89](ec0ba89)) * initial import ([d49fff6](d49fff6)) ### Bug Fixes * add CORS plugin ([5913f9d](5913f9d)) * conform to Delegated Routing V1 HTTP spec ([#41](#41)) ([41e7902](41e7902)) * conform to peer schema ([35f47da](35f47da)) * handle unparsable peer ids ([#118](#118)) ([9bdbe46](9bdbe46)) * increase listeners to silence node warnings ([#112](#112)) ([13f4084](13f4084)) * increase shutdown controller signal listeners ([#62](#62)) ([ab7afa7](ab7afa7)) * mark package as side-effect free ([551e0f2](551e0f2)) * PeerRecord Addrs and Protocols do not need to be optional ([#43](#43)) ([ec62768](ec62768)) * remove @helia/interface dep ([#59](#59)) ([aa8ffb8](aa8ffb8)) ### Trivial Changes * add or force update .github/workflows/js-test-and-release.yml ([#19](#19)) ([1af4a7a](1af4a7a)) * add project field to eslint config ([#40](#40)) ([7dac713](7dac713)) * add typedoc entry point ([5b16a12](5b16a12)) * delete templates [skip ci] ([#18](#18)) ([e1771ee](e1771ee)) * fix deps ([9aa177d](9aa177d)) * fix package name ([841d8f1](841d8f1)) * fix package name ([cd172da](cd172da)) * fix package name ([2c13d74](2c13d74)) * **release:** 1.0.0 [skip ci] ([a5b8290](a5b8290)), closes [#41](#41) [#40](#40) [#26](#26) [#8](#8) [#39](#39) [#26](#26) [#8](#8) [#39](#39) [#42](#42) * **release:** 1.0.0 [skip ci] ([b027b54](b027b54)), closes [#41](#41) [#40](#40) [#26](#26) [#8](#8) [#39](#39) [#26](#26) [#8](#8) [#39](#39) * **release:** 1.0.0 [skip ci] ([fd4871b](fd4871b)) * **release:** 1.0.0 [skip ci] ([867feb8](867feb8)) * **release:** 1.0.0 [skip ci] ([ed31f44](ed31f44)) * **release:** 1.0.1 [skip ci] ([39c92a5](39c92a5)), closes [#47](#47) * **release:** 1.0.1 [skip ci] ([58d76be](58d76be)), closes [#43](#43) * **release:** 1.0.1 [skip ci] ([7d3d708](7d3d708)), closes [#26](#26) [#8](#8) * **release:** 1.0.1 [skip ci] ([4a7be80](4a7be80)) * **release:** 1.0.2 [skip ci] ([4214634](4214634)), closes [#48](#48) * **release:** 1.0.2 [skip ci] ([8760907](8760907)), closes [#39](#39) * **release:** 1.0.2 [skip ci] ([9b4b3e8](9b4b3e8)), closes [#26](#26) [#8](#8) * **release:** 1.0.3 [skip ci] ([e8d3243](e8d3243)), closes [#49](#49) * **release:** 1.0.3 [skip ci] ([1aa75e3](1aa75e3)), closes [#39](#39) * **release:** 1.0.4 [skip ci] ([7e2bb0b](7e2bb0b)), closes [#58](#58) * **release:** 1.0.5 [skip ci] ([5341fc1](5341fc1)), closes [#59](#59) * **release:** 1.1.0 [skip ci] ([4f593ec](4f593ec)), closes [#44](#44) [#47](#47) * **release:** 1.1.1 [skip ci] ([9ab3df4](9ab3df4)), closes [#58](#58) * **release:** 1.1.2 [skip ci] ([1db87c0](1db87c0)), closes [#62](#62) * **release:** 2.0.0 [skip ci] ([c9936ca](c9936ca)), closes [#75](#75) [#100](#100) [#75](#75) [#94](#94) * **release:** 2.0.0 [skip ci] ([81281b6](81281b6)), closes [#75](#75) [#100](#100) [#75](#75) [#73](#73) [#77](#77) [#94](#94) * **release:** 2.0.1 [skip ci] ([814f9ef](814f9ef)), closes [#91](#91) * **release:** 2.0.1 [skip ci] ([7e4f169](7e4f169)), closes [#91](#91) * **release:** 2.0.2 [skip ci] ([01f6b44](01f6b44)) * **release:** 2.0.3 [skip ci] ([0fdea42](0fdea42)) * **release:** 2.1.0 [skip ci] ([e3a2d8f](e3a2d8f)), closes [#101](#101) * **release:** 3.0.0 [skip ci] ([be52e79](be52e79)), closes [#102](#102) * **release:** 3.0.0 [skip ci] ([d2624dc](d2624dc)), closes [#102](#102) * **release:** 3.0.1 [skip ci] ([a2f715a](a2f715a)), closes [#112](#112) * **release:** 3.0.1 [skip ci] ([5f2faf6](5f2faf6)), closes [#104](#104) * **release:** 3.0.2 [skip ci] ([6933d34](6933d34)), closes [#108](#108) * **release:** 3.0.3 [skip ci] ([bfbba0f](bfbba0f)), closes [#112](#112) * remove extra ci file ([#46](#46)) ([b130c17](b130c17)) * remove release from interop ([a3be11b](a3be11b)) * Update .github/pull_request_template.md [skip ci] ([c4fb76f](c4fb76f)) * Update .github/workflows/stale.yml [skip ci] ([30d5d80](30d5d80)) * Update .github/workflows/stale.yml [skip ci] ([a958569](a958569)) * Update .github/workflows/stale.yml [skip ci] ([b4f59b6](b4f59b6)) * update aegir ([c7c81b5](c7c81b5)) * update project config ([29bf459](29bf459)) * update project config ([#100](#100)) ([0bc6284](0bc6284)) * update sibling dependencies ([0c21765](0c21765)) * update sibling dependencies ([85cc65e](85cc65e)) * update sibling dependencies ([3fcc332](3fcc332)) * update sibling dependencies ([1b5d5dc](1b5d5dc)) ### Documentation * update project config and readmes ([#47](#47)) ([31eb6f7](31eb6f7)) * update readme examples to import correct symbols ([#58](#58)) ([bcfc785](bcfc785)) * update readmes and package descriptions ([11934bc](11934bc)) ### Dependencies * bump @fastify/cors from 8.5.0 to 9.0.1 ([#108](#108)) ([851b40f](851b40f)) * bump @libp2p/interface from 0.1.6 to 1.1.1 ([#91](#91)) ([50e4864](50e4864)) * bump helia from 1.3.12 to 2.0.1 ([#26](#26)) ([9160281](9160281)) * bump multiformats from 12.1.3 to 13.0.0 ([#75](#75)) ([1dabe16](1dabe16)) * bump p-queue from 7.4.1 to 8.0.1 ([#73](#73)) ([d575f73](d575f73)) * bump uint8arrays from 4.0.10 to 5.0.1 ([#77](#77)) ([e966c99](e966c99)) * **dev:** bump @helia/ipns from 5.0.0 to 6.0.0 ([#107](#107)) ([c4d9c8f](c4d9c8f)) * **dev:** bump @helia/ipns from 6.0.1 to 7.1.0 ([#110](#110)) ([bb10bf7](bb10bf7)) * **dev:** bump @types/sinon from 10.0.20 to 17.0.0 ([#49](#49)) ([c2d5bfb](c2d5bfb)) * **dev:** bump aegir from 39.0.13 to 40.0.8 ([#8](#8)) ([127bcc0](127bcc0)) * **dev:** bump aegir from 40.0.13 to 41.0.0 ([#39](#39)) ([c01a33e](c01a33e)) * **dev:** bump aegir from 41.3.5 to 42.1.1 ([#94](#94)) ([e34a142](e34a142)) * **dev:** bump helia from 3.0.1 to 4.0.0 ([#104](#104)) ([23cd638](23cd638)) * **dev:** bump sinon from 16.1.3 to 17.0.0 ([#42](#42)) ([43275c0](43275c0)) * **dev:** bump sinon-ts from 1.0.2 to 2.0.0 ([#48](#48)) ([7e0c7d3](7e0c7d3)) * update ipns to v9 ([#102](#102)) ([1097cb6](1097cb6)) * update sibling dependencies ([c2f7470](c2f7470)) * update sibling dependencies ([7d9cb15](7d9cb15))
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Enables passing a client instance as a libp2p service which will detect it's
ContentRouting
andPeerRouting
capabilities and configure them for use.Obviates the need for modules like @libp2p/delegated-routing-v1-http-api-content-routing.