Skip to content
This repository has been archived by the owner on Aug 21, 2020. It is now read-only.

Scope of rust-ipfs #8

Open
koivunej opened this issue Mar 3, 2020 · 11 comments
Open

Scope of rust-ipfs #8

koivunej opened this issue Mar 3, 2020 · 11 comments

Comments

@koivunej
Copy link

koivunej commented Mar 3, 2020

I propose the following:

  • We aim to build rust-ipfs as an embeddable crate initially for std capable rust applications which might want to expose the HTTP API in addition to working on the provided interfaces such as what the current Ipfs type provides
  • This crate should aim to support all operating system platforms (linux, windows, mac) as the rust-lang does, and currently compile on latest stable compiler. rust-ipfs will be build per grant to be testable with existing ipfs testing tools such as js-interface-http-api and interop
  • We will not offer default on experimental or partly implemented features in rust-ipfs and aim to pass the tests common tests in order not to fragment the ipfs API ecosystem but to be one additional implementation with different runtime characteristics and enable more ipfs projects in different programming languages
@aphelionz
Copy link
Contributor

We aim to build rust-ipfs as an embeddable crate initially for std capable rust applications which might want to expose the HTTP API in addition to working on the provided interfaces such as what the current Ipfs type provides

This makes sense, and if we agree on this I think a reasonable resolution to this as an issue would be to include it in the README and CONTRIBUTING.md. I can take ownership of that.

This crate should aim to support all operating system platforms (linux, windows, mac) as the rust-lang does, and currently compile on latest stable compiler. rust-ipfs will be build per grant to be testable with existing ipfs testing tools such as js-interface-http-api and interop

Agree on stable 👍 The resolution here is to enforce these standards via CI. Windows testing is the only one that's problematic, as it costs $$. Another goal for fundraising, perhaps.

We will not offer default on experimental or partly implemented features in rust-ipfs and aim to pass the tests common tests in order not to fragment the ipfs API ecosystem but to be one additional implementation with different runtime characteristics and enable more ipfs projects in different programming languages

@vmx Is there is an "official" designation for items in the IPFS spec? It would be great to have a very clear threshold for what we will support and what we will not.

Finally, @vmx @koivunej @dvc94ch and @rklaehn, here's a big open question: For the cli, following the command spec in lock-step makes sense. However for the Rust APIs, I'm wondering if the Rust language itself idiomatic (Traits and trait implementations are the main thing that comes to mind) in a way where the way traditional IPFS APIs are structured - ipfs.pin.ls(), ipfs.pin.add(), ipfs.pin.rm() - might be problematic?

@vmx
Copy link

vmx commented Mar 4, 2020

@vmx Is there is an "official" designation for items in the IPFS spec? It would be great to have a very clear threshold for what we will support and what we will not.

I think Rust IPFS should also implement experimental features (though not as a priority). Current experimental features of JS IPFS can be found here https://github.com/ipfs/js-ipfs/tree/e9eca18b9531f35b19094f9c0aa5073fba07814d/packages/ipfs#optionsexperimental.

I think eventually Rust IPFS should support all the things that the interop tests support.

I'm wondering if the Rust language itself idiomatic (Traits and trait implementations are the main thing that comes to mind) in a way where the way traditional IPFS APIs are structured - ipfs.pin.ls(), ipfs.pin.add(), ipfs.pin.rm() - might be problematic?

I would just start somewhere, but won't take decisions taken at the beginning for granted. I'd expect that things on earlier implemented commands might change again while implementing other commands.

@aphelionz
Copy link
Contributor

Another thing I noticed in going through the interface-js-ipfs-core tests is that some of them, for example https://github.com/ipfs/interface-js-ipfs-core/blob/f1077f710275ed6b222ecfa4357e956ddffc3ca8/src/refs-local.js. require that secondary endpoints be implemented that are not part of the grant proposal i.e. add

@vmx @hugomrdias your guidance would be very useful here. Luckily this looks like it might not be an issue until Phase 1.2 so we have some time to mitigate this

@hugomrdias
Copy link

Start looking into js-ipfs repo instead of that one, we are moving to a mono repo setup.
Regarding the tests send a PR refactoring those tests and we will review/merge asap.

@aphelionz
Copy link
Contributor

@hugomrdias Would it be possible to get this version of interface-ipfs-core published to npm? We currently don't need to patch the tests yet but it would give us the ^3.0.0 version of interface-ipfs-core we're after.

I can npm link for now but that would be awesome 🙏

@koivunej
Copy link
Author

koivunej commented Mar 9, 2020

Earlier @vmx wrote (sorry for the delay):

I think eventually Rust IPFS should support all the things that the interop tests support.

This sounds like a better scoping in the longer run. No point in not doing something if there are conformant impls and tests for that.

However for the Rust APIs, I'm wondering if the Rust language itself idiomatic (Traits and trait implementations are the main thing that comes to mind) in a way where the way traditional IPFS APIs are structured - ipfs.pin.ls(), ipfs.pin.add(), ipfs.pin.rm() - might be problematic?

I don't we should aim to keep the programming language apis the same, as priority at least. Multi-dot APIs could be offered but that seems like a waste in Rust. Some names and naming conventions have been bikeshed long already, for example: HashSet does not have add but insert, remove for rm and so on. Perhaps traits will allow similar division and place for cohesive documentation? I think we'll discover the APIs as we go and will tune them according to feedback.

@aphelionz
Copy link
Contributor

Perhaps we can eventually make some sort of document like "Rust IPFS for the [JS|Go] Developer"

@vmx
Copy link

vmx commented Mar 9, 2020

I don't we should aim to keep the programming language apis the same, as priority at least.

I agree. The same functionality should of course be implemented, but the API should be idiomatic for the corresponding language.

@koivunej
Copy link
Author

Looking at how the apis have evolved the Ipfs at the current is quite ugly (Ipfs::pubsub_* methods for example in rs-ipfs/rust-ipfs#118), the suggested refactorings (#119) will make this better once we can land those, I just wouldn't want to land them while we are trying to hurry up with the grant. Using traits no longer seems like a viable option since that'd all need to be #[async_trait]'d even though the current setup is not much better. I'd be ok for #[async_trait]'d approach if a non-async_trait way was possible but we'll need to think about this still.

One idea for non-async_trait access was to provide mostly libp2p Swarm alike API (leave polling of the "stream" or "everything" to user). This would work for applications which don't necessary want the multiple layers of tracking which are essential for conformance testing but not so much for any application, since the tracking cannot be used as a strong indicator but as a weak indicator at the level of "is a tcp connection open"..

Disregarding non-perfectly looking APIs I still feel that getting the common tests running is the number one priority since there is so much code already without conformance testing.

@dvc94ch
Copy link

dvc94ch commented Apr 1, 2020

Using traits no longer seems like a viable option since that'd all need to be #[async_trait]'d even though the current setup is not much better. I'd be ok for #[async_trait]'d approach if a non-async_trait way was possible but we'll need to think about this still.

What's wrong with #[async_trait]? It involves a heap allocation when calling an async function. So having an async init function is fine as it's only called once. All other async functions were replaced with non async versions in the suggested PR which should only have the overhead of a vtable lookup as I understand it.

@koivunej
Copy link
Author

koivunej commented Apr 2, 2020

... in the suggested PR ...

Just to clarify. In my previous comment I was replying to myself to my comment before that, specificly the idea I had with "splitting" or "namespacing" the single facade with traits, to allow for cohesive documentation.

@aphelionz aphelionz transferred this issue from rs-ipfs/rust-ipfs May 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants