Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12905 closed enhancement (implemented)

onionoo protocol version field

Reported by: iwakeh Owned by:
Priority: Medium Milestone:
Component: Metrics/Onionoo Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

While working on koninoo I noticed that the onionoo documents don't have a version field. (If I didn't miss anything obvious.)

It might be useful to add a field onionoo_version (the name doesn't matter) to the top level of all documents
delivered by onionoo, e.g.

{
"onionoo_version" : "1.0.7",
"relays_published":"2014-08-20 00:00:00",
"relays":[ ... ],
"bridges_published":"2014-08-20 00:00:00",
"bridges":[ ... ]
}

So, clients will not run into compatibility issues. Or, at least 'know' about this when parsing the version.

Child Tickets

Change History (10)

comment:2 Changed 5 years ago by iwakeh

Looks great, thanks!

You're right: minor and major version are sufficient.

In order to avoid future complaints from protocol clients,
I would suggest explaining the versioning in more detail:

minor version change

  • additional fields, parameters and/or request types
  • ommition of optional fields (like nickname)
  • ...

major version change

  • omittion of (previously deprecated) fields, parameters and/or request types
  • structural changes
  • ...

comment:3 Changed 5 years ago by karsten

Resolution: implemented
Status: needs_reviewclosed

Sounds good, I added two sentences explaining what's a minor and what's a major version change. Committed and deployed. Closing. Thanks!

comment:4 Changed 5 years ago by karsten

Resolution: implemented
Status: closedreopened

On second thought, this idea is great for clients to decide whether they are recent enough to parse a response. But if they're not, they are basically broken right then.

In theory, we can avoid that. We know in advance when we're going to make major protocol changes, and we typically give people a heads-up of 1 or 2 months before deploying those. Should we add another optional field "next_major_version" that contains an ISO date ("2014-10-01") and indicates when the next major protocol is going to happen?

comment:5 Changed 5 years ago by iwakeh

To tell the client about upcoming changes is an interesting approach!
I've not seen it used anywhere, but I like the idea.

Most clients usually won't work after a major version change and
adding the version field will enable them to inform their users
about the failure reason. That's way better than some unexplainable failure.

The information about the upcoming change enables the client
to inform its users about upcoming problems. So, there could be a warning
and suggestion to update the client software.

The ISO date format is fine.
Maybe name the field next_major_version_scheduled in order to make the
meaning clearer?

comment:6 Changed 5 years ago by karsten

Status: reopenedneeds_review

Please review my branch task-12905-2 which adds the suggested next_major_version_scheduled field. Thanks!

comment:7 Changed 5 years ago by iwakeh

Looks fine!

I think you forgot to remove the words "know whether they" from line 140 in protocol.html.

comment:8 Changed 5 years ago by karsten

Resolution: implemented
Status: needs_reviewclosed

Indeed, those three words look out of place there. I also changed the protocol version in the code to 1.1 and cleaned up the ChangeLog a bit. That should resolve this ticket. Closing again. Thanks!

comment:9 Changed 5 years ago by cypherpunks

from #tor-dev

18:05   chinguch1: karsten: why not version onionoo using the URL
18:06   chinguch1: karsten: onionoo.tpo/v1/summary
18:08   chinguch1: karsten: you can default the latest one with no version ex-onionoo.tpo/summary can be v2 and onionoo.tpo/v1/summary for clients that havent updated
18:09   chinguch1: or do the versioning with HTTP Accept
18:09   chinguch1: instead of doing this within the response object
18:09   chinguch1: which breaks everything

comment:10 Changed 5 years ago by karsten

cypherpunks/chinguch1, what you suggest would require serving multiple versions at once, which was never done before and which is currently not planned. I'm not saying that it's a bad idea, but it can be difficult to implement, depending on the protocol change.

Note that we're only making it more explicit now that a protocol change might break clients. We already had to make backward-incompatible changes in the past. We announced those changes months in advance and gave developers the opportunity to adapt their code. So far, I didn't hear about problems with this policy.

And technically, clients will not automatically break by a major protocol version bump. It's just that it's their fault if they break, not the server's fault.

Anyway, thanks for the input!

Note: See TracTickets for help on using tickets.