-
Notifications
You must be signed in to change notification settings - Fork 1.2k
The one true API, the ipfsx upgrade path #2509
Comments
The current implementation of ipfsx is pulling in the raw codecs from IPLD. You should probably just move to the new You may also want to consider returning/yielding |
If you plan to continue to use the |
It would probably be reimplemented using the API in |
This PR allows ipfsx to be used by calling `IPFS.create(options)` with `{ EXPERIMENTAL: { ipfsx: true } }` options. It adds a single API method `add` that returns an iterator that yields objects of the form `{ cid, path, size }`. The iterator is decorated with a `first` and `last` function so users can conveniently `await` on the first or last item to be yielded as per the [proposal here](https://github.com/ipfs-shipyard/ipfsx/blob/master/API.md#add). In order to boot up a new ipfsx node I refactored the boot procedure to enable the following: 1. **Remove the big stateful blob "`self`" - components are passed just the dependencies they need to operate.** Right now it is opaque as to which components require which parts of an IPFS node without inspecting the entirety of the component's code. This change makes it easier to look at a component and know what aspects of the IPFS stack it uses and consequently allows us to understand which APIs should be available at which points of the node's lifecycle. It makes the code easier to understand, more maintainable and easier to mock dependencies for unit tests. 1. **Restrict APIs to appropriate lifecycle stage(s).** This PR introduces an `ApiManager` that allows us to update the API that is exposed at any given point. It allows us to (for example) disallow `ipfs.add` before the node is initialized or access `libp2p` before the node is started. The lifecycle methods `init`, `start` and `stop` each define which API methods are available after they have run avoiding having to put boilerplate in every method to check if it can be called when the node is in a particular state. See #1438 1. **Safer and more flexible API usage.** The `ApiManager` allows us to temporarily change APIs to stop `init` from being called again while it is already running and has the facility to rollback to the previous API state if an operation fails. It also enables piggybacking so we don't attempt 2 or more concurrent start/stop calls at once. See #1061 #2257 1. **Enable config changes at runtime.** Having an API that can be updated during a node's lifecycle will enable this feature in the future. **FEEDBACK REQUIRED**: The changes I've made here are a little...racy. They have a bunch of benefits, as I've outlined above but the `ApiManager` is implemented as a `Proxy`, allowing us to swap out the underlying API at will. How do y'all feel about that? Is there a better way or got a suggestion? resolves #1438 resolves #1061 resolves #2257 refs #2509 refs #1670 License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
I'm not onboard with this. The hypothesis that giving yet another API as an option in addition to all the previous ones will benefit the users should not be assumed. I see it missing to recognize the amount of understanding necessary to even grok why these exist, why one is better than the other and why would a user need to care. Creating a great API is like creating a great product, there needs to be a lot of "no" involved otherwise it is just a compromise soup. We've already agreed both in person (Berlin Dev Meetings and IPFS Camp + other hack weeks) and async (https://github.com/ipfs/interface-js-ipfs-core/issues/284) that the Files API needs to be refactored and that APIs such as Block & Object can either be deprecated or hidden. If you want to explore a migrate path such as this (#2509), consider integrating in that migrate path something that clearly routes the users into a more happy place, rather than giving even more options and expecting them to differentiate well (specially when there are almost no docs). My vote goes for having the new API in beta and call the old one legacy to make it evident that users should move away from it (or something that clearly points to the user what is new and good and what is old and should change). |
IMHO, releasing under the EXPERIMENTAL flag would give us the licence to release an incomplete core API, that may have to change before it becomes the blessed core API. I take your point on adding another API though, and on reflection I think you're correct that we're not clearly moving users towards a more happy place in this way. At least, it would require a significant effort on our part to comminucate that The problem I'm trying to solve is that adding ipfsx APIs is going to take time - we can't send a single PR with all the changes, it has to be done incrementally. I also don't think it's really an option for us to have a main release that actually removes functionality without an upgrade path (unless it's a planned removal of an API). We could say We'd still use On a separate note I think "beta" is a loaded term and ideally I'd like the HTTP and CLI APIs to also be revamped before we move js-ipfs to beta. Also, one final assertion - I'm not going to release anything related to these API changes that doesn't have great documentation and useful examples to accompany it 😁 |
It’s a little odd as a consumer to have a legacy API, a Beta API, and no “current” or “stable” API. I also think it’s just generally a good practice to put out new things early, gather feedback, iterate, and then once they stabilize consider replacing the existing API. No matter how good an idea is, and I think this new API is a great idea, you still want to have a discussion about replacing the current API based on the full implementation and feedback it has received rather than just replacing with a beta API that hasn’t been used in the wild much yet. |
The point of having
Agreed - the current API would continue to be the current API, until We can't really tell people to move off the current API until there's something for them to move to, and it has good answers for the majority of use cases.
Rather than it having different code, it might just have the
This should be standard practise for everything we do 🚀 |
This PR allows ipfsx to be used by calling `IPFS.create(options)` with `{ EXPERIMENTAL: { ipfsx: true } }` options. It adds a single API method `add` that returns an iterator that yields objects of the form `{ cid, path, size }`. The iterator is decorated with a `first` and `last` function so users can conveniently `await` on the first or last item to be yielded as per the [proposal here](https://github.com/ipfs-shipyard/ipfsx/blob/master/API.md#add). In order to boot up a new ipfsx node I refactored the boot procedure to enable the following: 1. **Remove the big stateful blob "`self`" - components are passed just the dependencies they need to operate.** Right now it is opaque as to which components require which parts of an IPFS node without inspecting the entirety of the component's code. This change makes it easier to look at a component and know what aspects of the IPFS stack it uses and consequently allows us to understand which APIs should be available at which points of the node's lifecycle. It makes the code easier to understand, more maintainable and easier to mock dependencies for unit tests. 1. **Restrict APIs to appropriate lifecycle stage(s).** This PR introduces an `ApiManager` that allows us to update the API that is exposed at any given point. It allows us to (for example) disallow `ipfs.add` before the node is initialized or access `libp2p` before the node is started. The lifecycle methods `init`, `start` and `stop` each define which API methods are available after they have run avoiding having to put boilerplate in every method to check if it can be called when the node is in a particular state. See #1438 1. **Safer and more flexible API usage.** The `ApiManager` allows us to temporarily change APIs to stop `init` from being called again while it is already running and has the facility to rollback to the previous API state if an operation fails. It also enables piggybacking so we don't attempt 2 or more concurrent start/stop calls at once. See #1061 #2257 1. **Enable config changes at runtime.** Having an API that can be updated during a node's lifecycle will enable this feature in the future. **FEEDBACK REQUIRED**: The changes I've made here are a little...racy. They have a bunch of benefits, as I've outlined above but the `ApiManager` is implemented as a `Proxy`, allowing us to swap out the underlying API at will. How do y'all feel about that? Is there a better way or got a suggestion? resolves #1438 resolves #1061 resolves #2257 refs #2509 refs #1670 License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
This PR allows ipfsx to be used by calling `IPFS.create(options)` with `{ EXPERIMENTAL: { ipfsx: true } }` options. It adds a single API method `add` that returns an iterator that yields objects of the form `{ cid, path, size }`. The iterator is decorated with a `first` and `last` function so users can conveniently `await` on the first or last item to be yielded as per the [proposal here](https://github.com/ipfs-shipyard/ipfsx/blob/master/API.md#add). In order to boot up a new ipfsx node I refactored the boot procedure to enable the following: 1. **Remove the big stateful blob "`self`" - components are passed just the dependencies they need to operate.** Right now it is opaque as to which components require which parts of an IPFS node without inspecting the entirety of the component's code. This change makes it easier to look at a component and know what aspects of the IPFS stack it uses and consequently allows us to understand which APIs should be available at which points of the node's lifecycle. It makes the code easier to understand, more maintainable and easier to mock dependencies for unit tests. 1. **Restrict APIs to appropriate lifecycle stage(s).** This PR introduces an `ApiManager` that allows us to update the API that is exposed at any given point. It allows us to (for example) disallow `ipfs.add` before the node is initialized or access `libp2p` before the node is started. The lifecycle methods `init`, `start` and `stop` each define which API methods are available after they have run avoiding having to put boilerplate in every method to check if it can be called when the node is in a particular state. See #1438 1. **Safer and more flexible API usage.** The `ApiManager` allows us to temporarily change APIs to stop `init` from being called again while it is already running and has the facility to rollback to the previous API state if an operation fails. It also enables piggybacking so we don't attempt 2 or more concurrent start/stop calls at once. See #1061 #2257 1. **Enable config changes at runtime.** Having an API that can be updated during a node's lifecycle will enable this feature in the future. **FEEDBACK REQUIRED**: The changes I've made here are a little...racy. They have a bunch of benefits, as I've outlined above but the `ApiManager` is implemented as a `Proxy`, allowing us to swap out the underlying API at will. How do y'all feel about that? Is there a better way or got a suggestion? resolves #1438 resolves #1061 resolves #2257 refs #2509 refs #1670 License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
A roundup of the following PRs: * closes #2658 * closes #2660 * closes #2661 * closes #2668 * closes #2674 * closes #2676 * closes #2680 * test fixes and other fix ups --- To allow us to pass the interface tests, the timeout option is now supported in the `object.get` and `refs` APIs. It doesn't actually cancel the operation all the way down the stack, but allows the method call to return when the timeout is reached. https://github.com/ipfs/js-ipfs/pull/2683/files#diff-47300e7ecd8989b6246221de88fc9a3cR170 --- Supersedes #2724 --- resolves #1438 resolves #1061 resolves #2257 resolves #2509 resolves #1670 refs ipfs-inactive/interface-js-ipfs-core#394 BREAKING CHANGE: `IPFS.createNode` removed BREAKING CHANGE: IPFS is not a class that can be instantiated - use `IPFS.create`. An IPFS node instance is not an event emitter. BREAKING CHANGE: Instance `.ready` property removed. Please use `IPFS.create` instead. BREAKING CHANGE: Callbacks are no longer supported on any API methods. Please use a utility such as [`callbackify`](https://www.npmjs.com/package/callbackify) on API methods that return Promises to emulate previous behaviour. BREAKING CHANGE: `PeerId` and `PeerInfo` classes are no longer statically exported from `ipfs-http-client` since they are no longer used internally. BREAKING CHANGE: `pin.add` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property. BREAKING CHANGE: `pin.ls` now returns an async iterable. BREAKING CHANGE: `pin.ls` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property. BREAKING CHANGE: `pin.rm` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property. BREAKING CHANGE: `add` now returns an async iterable. BREAKING CHANGE: `add` results now contain a `cid` property (a [CID instance](https://github.com/multiformats/js-cid)) instead of a string `hash` property. BREAKING CHANGE: `addReadableStream`, `addPullStream` have been removed. BREAKING CHANGE: `ls` now returns an async iterable. BREAKING CHANGE: `ls` results now contain a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property. BREAKING CHANGE: `files.readPullStream` and `files.readReadableStream` have been removed. BREAKING CHANGE: `files.read` now returns an async iterable. BREAKING CHANGE: `files.lsPullStream` and `files.lsReadableStream` have been removed. BREAKING CHANGE: `files.ls` now returns an async iterable. BREAKING CHANGE: `files.ls` results now contain a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property. BREAKING CHANGE: `files.ls` no longer takes a `long` option (in core) - you will receive all data by default. BREAKING CHANGE: `files.stat` result now contains a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `hash` property. BREAKING CHANGE: `get` now returns an async iterable. The `content` property value for objects yielded from the iterator is now an async iterable that yields [`BufferList`](https://github.com/rvagg/bl) objects. BREAKING CHANGE: `stats.bw` now returns an async iterable. BREAKING CHANGE: `addFromStream` has been removed. Use `add` instead. BREAKING CHANGE: `addFromFs` has been removed. Please use the exported `globSource` utility and pass the result to `add`. See the [glob source documentation](https://github.com/ipfs/js-ipfs-http-client#glob-source) for more details and an example. BREAKING CHANGE: `addFromURL` has been removed. Please use the exported `urlSource` utility and pass the result to `add`. See the [URL source documentation](https://github.com/ipfs/js-ipfs-http-client#url-source) for more details and an example. BREAKING CHANGE: `name.resolve` now returns an async iterable. It yields increasingly more accurate resolved values as they are discovered until the best value is selected from the quorum of 16. The "best" resolved value is the last item yielded from the iterator. If you are interested only in this best value you could use `it-last` to extract it like so: ```js const last = require('it-last') await last(ipfs.name.resolve('/ipns/QmHash')) ``` BREAKING CHANGE: `block.rm` now returns an async iterable. BREAKING CHANGE: `block.rm` now yields objects of `{ cid: CID, error: Error }`. BREAKING CHANGE: `dht.findProvs`, `dht.provide`, `dht.put` and `dht.query` now all return an async iterable. BREAKING CHANGE: `dht.findPeer`, `dht.findProvs`, `dht.provide`, `dht.put` and `dht.query` now yield/return an object `{ id: CID, addrs: Multiaddr[] }` instead of a `PeerInfo` instance(s). BREAKING CHANGE: `refs` and `refs.local` now return an async iterable. BREAKING CHANGE: `object.data` now returns an async iterable that yields `Buffer` objects. BREAKING CHANGE: `ping` now returns an async iterable. BREAKING CHANGE: `repo.gc` now returns an async iterable. BREAKING CHANGE: `swarm.peers` now returns an array of objects with a `peer` property that is a `CID`, instead of a `PeerId` instance. BREAKING CHANGE: `swarm.addrs` now returns an array of objects `{ id: CID, addrs: Multiaddr[] }` instead of `PeerInfo` instances. BREAKING CHANGE: `block.stat` result now contains a `cid` property (whose value is a [CID instance](https://github.com/multiformats/js-cid)) instead of a `key` property. BREAKING CHANGE: `bitswap.wantlist` now returns an array of [CID](https://github.com/multiformats/js-cid) instances. BREAKING CHANGE: `bitswap.stat` result has changed - `wantlist` and `peers` values are now an array of [CID](https://github.com/multiformats/js-cid) instances. BREAKING CHANGE: the `init` option passed to the IPFS constructor will now not take any initialization steps if it is set to `false`. Previously, the repo would be initialized if it already existed. This is no longer the case. If you wish to initialize a node but only if the repo exists, pass `init: { allowNew: false }` to the constructor. BREAKING CHANGE: removed `file ls` command from the CLI and HTTP API. BREAKING CHANGE: Delegated peer and content routing modules are no longer included as part of core (but are still available if starting a js-ipfs daemon from the command line). If you wish to use delegated routing and are creating your node _programmatically_ in Node.js or the browser you must `npm install libp2p-delegated-content-routing` and/or `npm install libp2p-delegated-peer-routing` and provide configured instances of them in [`options.libp2p`](https://github.com/ipfs/js-ipfs#optionslibp2p). See the module repos for further instructions: - https://github.com/libp2p/js-libp2p-delegated-content-routing - https://github.com/libp2p/js-libp2p-delegated-peer-routing
But what does that mean though?
Something that bothers me is that we already support promises & callbacks in the API, so removing callbacks doesn't really give our users anything except a whole bunch of refactoring for those who do used the callbackified API. That can't be the end goal of #1670.
I think we should get on and implement
ipfsx
.We've seen the API grow organically over time and it doesn't present the most streamlined interface. The key point is that IPFS is all about files, yet loads of the file operations are not top-level operations. Plus we have confusing caveat-filled weirdness with things like
ipfs.ls
andipfs.files.ls
.The
ipfsx
approach is to finally unify the files API and move them all to the top level front and centre and have the low level stuff grouped by domain (e.g.block
,dag
, etc). It also removes readable streams, pull streams, etc giving us a smaller, more focused API surface area and (hopefully) smaller bundle size. I think we're all agreed that this is where we're heading.As the internals follow the path of
ipfs._addAsyncIterator
we can start to implement ipfsx on top of them, then eventually split out the current API as a separate module to ease upgrading for people.Step 1
Allow the user to expose the new API as an experimental flag. The old API will not be exposed - this may mean that some operations are impossible until they are implemented. Welcome to the bleeding edge.
Step 2
Once the API is implemented, remove the old API entirely, deps and all. This should give us a leaner bundle and small surface area for those who have kept up to date.
Then create a wrapper for the new API that implements the old by calling new API methods and is callbackified/promisified/promise-nodified. This gives existing users a smooth upgrade path.
We should be able to verify it using today's
interface-ipfs-core
suite, though we should make tomorrow's version will only test theipfsx
interface.The text was updated successfully, but these errors were encountered: