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

Fix Apiary spec and align it with js-ipfs API #122

Closed
hsanjuan opened this issue Oct 19, 2016 · 8 comments
Closed

Fix Apiary spec and align it with js-ipfs API #122

hsanjuan opened this issue Oct 19, 2016 · 8 comments
Assignees

Comments

@hsanjuan
Copy link
Member

This issue corresponds to the "API alignment" milestone: ipfs/team-mgmt#227

A quick look upon the blueprint shows a number of issues (curl command used as expected request body etc). Additionally, the spec should be completely aligned to the js-ipfs implementation, which needs to be checked.

There are a number of open issues regarding the API which might be addressed in this effort. I'll be linking as I go,

@RichardLitt
Copy link
Contributor

curl command used as expected request body

Where is this happening?

@victorb
Copy link

victorb commented Oct 19, 2016

@RichardLitt in many places, first example I can find here: https://github.com/ipfs/http-api-spec/blob/91922f0f4a400d279d2a8a009475c44c1a96e95c/apiary.apib#L36-L40

body is supposed to be the response body from the API call rather than the command that would return the body.

@RichardLitt
Copy link
Contributor

Fair. I'm not sure why I did this - can't find the documentation in the past issues. Would be good to fix. A lot of this code was influenced by the fact that I couldn't figure out how to do it automatically, or wasn't.

@hsanjuan
Copy link
Member Author

Status report: :(

go-ipfs API implementation seems to be built on top of the CLI commands (or maybe actual Core API). But operations do not translate very well to HTTP endpoints in some cases.

js-ipfs seems to have implemented Go endpoints, except it usually offers less functionality than Go regarding options and operations modifiers.

Headers and return objects and values differ among implementations sometimes. There is no consistency regarding which Content Type this API uses (text plain sometimes, json others etc).

HTTP API tries to be an RPC API and is by no means a REST API. Apiary (or Swagger) are designed to document APIs which try to be RESTful, making the process more painful and generating suboptimal documentation. There are also some endpoints which are impossible to accurately document right now because they're just weird (things taking ?arg=key&arg=value for example).

Because of this, there is little possibility (IMHO) that we can use API docs to auto-generate any serious test suite (let alone to test both implementations). Doing it is going to require to fight with lots of quirks.

Therefore I'm in the process of thinning the apiary doc into a minimal usable set for the users that works both for js-ipfs and go-ipfs and that is not as long as now (longer -> more maintenance). That is:

  • Get apiary to generate code/curl for the main endpoint example correctly and have apiary fake-api endpoints behave as the ipfs daemon would where its possible.
  • Drop examples about calls with wrong parameters (where we mostly return 500s, which is also wrong and confusing) and example which just show flags being used with different values (apiary is not well suited for even this in RPC-like APIs)
    • Write down inconsistencies for each endpoint

I'll bring this discussion to the next all hands. Questions open:

  • Is Apiary the right format/place to document our type of API?
  • v1 HTTP API: Decoupling HTTP API from Core and go for a strict RESTful, or JSON-RPC.

Linking related issues:
#108
#116

@daviddias
Copy link
Contributor

daviddias commented Oct 30, 2016

go-ipfs API implementation seems to be built on top of the CLI commands (or maybe actual Core API). But operations do not translate very well to HTTP endpoints in some cases.

go-ipfs 'networked API' (currently over HTTP) was designed to be an API that is friendly to be mounted over any pipe/socket, then, due to browser requirements (more like restrictions) it had to be hammered into what we have today as an HTTP API

Lots of very important context: https://github.com/ipfs/http-api-spec/issues/116

js-ipfs seems to have implemented Go endpoints, except it usually offers less functionality than Go regarding options and operations modifiers.

They are just not implemented yet

HTTP API tries to be an RPC API and is by no means a REST API. Apiary (or Swagger) are designed to document APIs which try to be RESTful, making the process more painful and generating suboptimal documentation. There are also some endpoints which are impossible to accurately document right now because they're just weird (things taking ?arg=key&arg=value for example).

One of the big questions is, can we really have a canonical API for RPC and REST? Battle experience says no.

Thank you for working on this, it has been a huge pain. Keep it coming :D

@hsanjuan
Copy link
Member Author

hsanjuan commented Nov 3, 2016

After our monday conversation and the pain of manually dealing with Apiary I opted for writing a tool to simply generate the docs from Go.

This materializes in the above's pull request (ipfs-inactive/website#156) and deprecates Apiary (and this repo) by removing the references from the website.

By autogenerating the API reference directly from go code we get:

  • No maintenance costs and hunting people down to keep docs up to date
  • Nice documentation for developers which is actually useful and integrated with the website

It provides a good base for addressing stuff in #116 . For the moment I'm considering Apiary spec "fixed".

@hsanjuan hsanjuan changed the title Fix Apiary spec and align it spec with js-ipfs API Fix Apiary spec and align it with js-ipfs API Nov 3, 2016
@RichardLitt
Copy link
Contributor

I opted for writing a tool to simply generate the docs from Go.

This sounds to me like the right approach. I wish we had more of a canonical source to look at, however, as this won't catch differences in the go implementation that should be in every IPFS implementation. However, it is probably a better use of our tooling to do this. I think that the ipfs/specs should cover what I want, anyway.

No maintenance costs and hunting people down to keep docs up to date

I like this.

Deprecating this repo sounds fine. We should pull out any issues that we want covered, though, before closing them and putting a notice up. We should also remove references to the HTTP API.

@RichardLitt
Copy link
Contributor

Also, good work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants