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

Redesign API for respondd #522

Open
tcatm opened this Issue Oct 12, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@tcatm

tcatm commented Oct 12, 2015

No description provided.

@jplitza

This comment has been minimized.

Show comment
Hide comment
@jplitza

jplitza Oct 16, 2015

Member

I suppose this is the continuation of the discussion we had in #465?

Member

jplitza commented Oct 16, 2015

I suppose this is the continuation of the discussion we had in #465?

@NeoRaider NeoRaider added this to the 2015.2 milestone Oct 16, 2015

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Oct 16, 2015

Member

@jplitza, yes. We'd like to get this done before 2015.2, so we can get rid of the old API as soon as possible.

Member

NeoRaider commented Oct 16, 2015

@jplitza, yes. We'd like to get this done before 2015.2, so we can get rid of the old API as soon as possible.

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Oct 27, 2015

Member

Suggestion: use JSON for the requests. Draft:

{
  "keys": ["nodeinfo"],
  "single": true,
  "compress": true
}
  • keys: lists the keys to get (so multiple types can be requested with a single request, as supported by the current GET API)
  • single: can be set to true if keys contains only a single key to send a reply without the key wrapper (see the current GET nodeinfo vs. nodeinfo requests to see the difference). Defaults to false. Do we even need this? The only users of the old API are the old statuspage (which won't support the new API anyways) and piping the data to alfred.
  • compress: obvious. Defaults to false?
Member

NeoRaider commented Oct 27, 2015

Suggestion: use JSON for the requests. Draft:

{
  "keys": ["nodeinfo"],
  "single": true,
  "compress": true
}
  • keys: lists the keys to get (so multiple types can be requested with a single request, as supported by the current GET API)
  • single: can be set to true if keys contains only a single key to send a reply without the key wrapper (see the current GET nodeinfo vs. nodeinfo requests to see the difference). Defaults to false. Do we even need this? The only users of the old API are the old statuspage (which won't support the new API anyways) and piping the data to alfred.
  • compress: obvious. Defaults to false?
@jplitza

This comment has been minimized.

Show comment
Hide comment
@jplitza

jplitza Oct 31, 2015

Member

I am getting the impression we are re-inventing (read-only) HTTP over UDP here.

JSON would have the advantage of being forward-compatible: We could add further flags in the future without breaking old applications. But it is also very verbose: What you posted is four times longer than "GETZ nodeinfo". As you mentioned, "single" isn't really required, so what advantage does JSON give us right now?

(also: did you abandon the idea of partitioning the answers?)

Member

jplitza commented Oct 31, 2015

I am getting the impression we are re-inventing (read-only) HTTP over UDP here.

JSON would have the advantage of being forward-compatible: We could add further flags in the future without breaking old applications. But it is also very verbose: What you posted is four times longer than "GETZ nodeinfo". As you mentioned, "single" isn't really required, so what advantage does JSON give us right now?

(also: did you abandon the idea of partitioning the answers?)

@tcatm

This comment has been minimized.

Show comment
Hide comment
@tcatm

tcatm Nov 2, 2015

Well, let's start by looking at the current state. There are two types of requests:

a) GET id [id...] for requesting a bunch of records as compressed json
b) id for requesting a single record, uncompressed, raw json

These requests are used like this:

b) is used by the old status page just with the id set to "nodeinfo". We'll need to keep this around for backward compatibility a while.
b) is also used locally as a replacement for collect.lua id. This time, other ids like "statistics" and "neighbours" are also queried. This interface can be changed at any time so it shouldn't pose a problem.
a) was introduced in 2015.1 as a stable API. There may be users relying on it so we should keep it for a few releases, too.
a) is used by the new status-page. We could change that before releasing 2015.2.

If we were to introduce a new API in 2015.2, we still must support the old APIs for a while, giving us a total of three public APIs.

Therefore, I'd like to propose not changing anything in 2015.2 but to deprecate the public part of b) now and schedule its removal for 2015.2+1. A few releases later we could replace a) the same way (or transition to NetJSON rightaway).

tcatm commented Nov 2, 2015

Well, let's start by looking at the current state. There are two types of requests:

a) GET id [id...] for requesting a bunch of records as compressed json
b) id for requesting a single record, uncompressed, raw json

These requests are used like this:

b) is used by the old status page just with the id set to "nodeinfo". We'll need to keep this around for backward compatibility a while.
b) is also used locally as a replacement for collect.lua id. This time, other ids like "statistics" and "neighbours" are also queried. This interface can be changed at any time so it shouldn't pose a problem.
a) was introduced in 2015.1 as a stable API. There may be users relying on it so we should keep it for a few releases, too.
a) is used by the new status-page. We could change that before releasing 2015.2.

If we were to introduce a new API in 2015.2, we still must support the old APIs for a while, giving us a total of three public APIs.

Therefore, I'd like to propose not changing anything in 2015.2 but to deprecate the public part of b) now and schedule its removal for 2015.2+1. A few releases later we could replace a) the same way (or transition to NetJSON rightaway).

@NeoRaider NeoRaider modified the milestones: next, 2015.2 Nov 3, 2015

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Nov 3, 2015

Member

@tcatm, one thing I forgot in my JSON draft, but deem quite important is the "partitioned query" issue. I'd like to get this sorted out as soon as possible, as I'd like to prevent people from actually starting to use respondd via mesh-wide multicast (packet storms are never a nice thing...).

Is the "neighbour name" feature on the old status page important enough to keep supporting the old API at all?

Member

NeoRaider commented Nov 3, 2015

@tcatm, one thing I forgot in my JSON draft, but deem quite important is the "partitioned query" issue. I'd like to get this sorted out as soon as possible, as I'd like to prevent people from actually starting to use respondd via mesh-wide multicast (packet storms are never a nice thing...).

Is the "neighbour name" feature on the old status page important enough to keep supporting the old API at all?

@tcatm

This comment has been minimized.

Show comment
Hide comment
@tcatm

tcatm Nov 3, 2015

There is already software relying on the GET-API: https://github.com/dereulenspiegel/node-informant

I'm not sure whether the neighbour name feature is important but it shouldn't do any harm keeping support for it around for at least one release.

tcatm commented Nov 3, 2015

There is already software relying on the GET-API: https://github.com/dereulenspiegel/node-informant

I'm not sure whether the neighbour name feature is important but it shouldn't do any harm keeping support for it around for at least one release.

@NeoRaider

This comment has been minimized.

Show comment
Hide comment
@NeoRaider

NeoRaider Feb 10, 2016

Member

@dereulenspiegel is working on a redesigned respondd based on CoAP. I guess we can live with the current respondd API until that is done.

Member

NeoRaider commented Feb 10, 2016

@dereulenspiegel is working on a redesigned respondd based on CoAP. I guess we can live with the current respondd API until that is done.

@NeoRaider NeoRaider changed the title from Redesign API for announced to Redesign API for respondd Feb 10, 2016

@NeoRaider NeoRaider modified the milestones: next, 2016.2 Feb 16, 2016

@genofire genofire referenced this issue Mar 19, 2016

Closed

JSON redesign #28

@rotanid rotanid added enhancement needs work RFC and removed RFC labels Aug 22, 2016

@rotanid rotanid added known issue and removed needs work labels Jun 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment