TheThingsArchive / api

Generated code for accessing The Things Network v2 API
MIT License
12 stars 8 forks source link

Changes to monitor proto #6

Closed rvolosatovs closed 7 years ago

rvolosatovs commented 7 years ago

I propose 3 changes to monitor proto:

  1. Let's rename GatewayUplink and GatewayDownlink to RouterUplink/RouterDownlink respectively, to better reflect the source of data and type.
  2. In current implementation ttn opens 3 streams per component(gateway,router,handler,broker) at all times. Let's open only 1 stream per component and wrap message in a oneof Status,Uplink or Downlink. This makes handling much easier.
  3. Let's add all missing endpoints to the components, in other words, let's make sure there are 3 endpoints per each component(or 1, if we agree on my previous point)
htdvisser commented 7 years ago
  1. Renaming this will break existing clients, so that's not going to happen (gRPC service endpoints are addressed by name).
  2. That shouldn't make much of a difference, but we could add a "composite" stream in the future. Not really a priority imo.
  3. Not all endpoints make sense. For example, the NetworkServer component is mostly just a database and doesn't really add additional information compared to the Broker

Considering that we're also thinking about merging some components (router,broker,networkserver) I don't see why we would make these changes.

rvolosatovs commented 7 years ago
  1. This is 2 calls to gorename in ttn repo, 1 call to gorename in NOC repo. Which clients are going to be broken? TTN will just get an error about an unimplemented endpoint for a couple of seconds.
  2. Given the functionality we want, it does. It's much easier to count uptime that way, if we rely on stream status, also why would gateway authenticate 3 times if 1 time is enough?
  3. ok
htdvisser commented 7 years ago

Please keep in mind that are APIs are considered stable for the v2 version. We can't just go around and change things because we realize that it wasn't perfect.

rvolosatovs commented 7 years ago

can we do this for v3 then?

htdvisser commented 7 years ago

Maybe. Iceboxing this issue