search

MetPX / wmo_mesh

minimal sample to demonstrate mesh network with a pub/sub message passing protocol (mqtt in this case.)
GNU General Public License v2.0
4 stars 2 forks source link

Change from JSON tuple to JSON object #6

Closed petersilva closed 4 years ago

petersilva commented 5 years ago

At the meeting, reviewers suggested that a JSON object type be explicitly used for data representation. So instead of::

[ pubtime, baseUrl, relPath, headers ]

the body becomes::

headers

and within the headers there are "pubTime", "baseUrl", and "relPath" name:value pairs.

The reasoning for the original tuple notation was:

reviewers like how it looks as one object... ok...

petersilva commented 5 years ago

So this is implemented, but I find it much harder to read than the original (probably because real-life examples have a many other fields, so it becomes difficult to follow in practice, when different implementations may sort things differently, or have different intervening fields.)

I was originally thinking of moving the sum (now called "integrity") to be a fourth mandatory field...

I think the original format (perhaps with the addition of the integrity header as a mandatory) is easier to work with and more compact. I prefer and recommend the original format, but perhaps I am odd. Other people should comment. It is easy to change back (while we have an experimental test bed... ) if people agree.

petersilva commented 5 years ago

I guess there is this subtle perspective to keep in mind: I want the protocol to be easy to debug/follow for linux/admin types, so making it easy to understand the flow going by is important. It isn't just for developers (who tolerate a bit more tooling), but sufficiently simple for operational telecom types, with the placement of the mandatory fields at the beginning, it made things easy to read in the json dumps as they go by...

josusky commented 5 years ago

I was about to commit the JSON schema we were working on during the meeting (with some refinements), but fortunately I have checked the issue page first. Now I am in doubt. I understand the readability point of view on the other hand relying on parameter order makes me feel uneasy. Perhaps as a programmer I am to inattentive to such details and therefore I have made too many mistakes :-)

petersilva commented 5 years ago

Please go ahead and commit what you have, it is easier to discuss if we can see it. If you don't want it considered definitive because of inconsistencies with what is the demo, then create a branch (suggested name: issue17) and we can track it there until happy to merge to master.

I did what the committee asked, it should not be a show stopper, and can always be revisited later in an optimization stage. I tallied up the changes in move to v3, and with all the increased clarity the cost is about 70 additional bytes per message (estimated, might be off a little.) Our main switch is doing 1500 messages per second, so that is 700 kb/s additional for the same flow... a bit of a shame. I figure this is just a place holder to consider if we want to optimize later, but would be premature to do so now.

For now, please just assume as was agreed there, and we can confirm functionality. I'm eager to see the JSON schema. i um... changed the name of "sum" to "integrity" to allow signatures to fit in the same thing, it might be good to make that change to the schema? (now you don't need choice of 'sum' or 'signature', just have 'integrity' always ) doing with the separate attributes was painful because we persist to disk with extended attributes, and looking for one or the other just added optional code paths for very limited value I could see.

There is also two other integrity algorithms that are in the Sarracenia stack, but not the simplified wmo_mesh one: L - link (value is sha512 sum of content of the link.) 'R' - file removed, value is SHA512 sum of the name of the file to remove. These are used to mirroring use case unrelated to WMO, but had to include the in Sarracenia to get through internal tests.

There is also one header in the Canadian stack which had been left out of discussions to get buy in on the basic idea... the committee people knew instantly it needed to be added, and started re-inventing meta data about file size... so I'd like to get that nailed down also... see issue #7 ... that is a relatively straight-forward way to include size info in messages, that I can migrate v02 functionality to with relative ease. Once that header is agreed, pretty much all of the functionality needed to scale up will be present. feedback needed.

josusky commented 4 years ago

Schema has been published some time ago and is being used. Open questions (like the checksum) are discussed in other issues.