Fix JSON-as-TChannel arg2 docs

uber / tchannel

network multiplexing and framing protocol for RPC

MIT License

1.15k stars 129 forks source link

Fix JSON-as-TChannel arg2 docs #1391

Closed willsalz closed 5 years ago

willsalz commented 8 years ago

Please see: https://github.com/uber/tchannel/pull/1206 and https://github.com/uber/tchannel-python/pull/187. I'm going out on a limb here, but I'm assuming that Node is not implemented in the same way…

cc @prashantv @breerly @junchaowu @Raynos

Raynos commented 8 years ago

:-1: If it's going to be a headers it should probably be JSON encoded headers, i.e. valid JSON but restricted to only a shallow object of key value pairs where they are strings.

prashantv commented 8 years ago

+1 to @Raynos's suggestion. Go treats headers as map<string,string> but encoded with JSON.

Raynos commented 8 years ago

cc @breerly

So we have the following:

node arg2 is any valid JSON
go arg2 is a map<string, string> encoded as JSON
pythong arg2 is a map<string, string> encoded as nh:2 (k~2 v~2){nh}
java arg2 is a map<string, string> encoded as JSON
tchannel spec; arg2 is a map<string, string> encoded as JSON

At this point we need to make a binary breaking change somewhere. I want to avoid making as many binary breaking changes as possible.

hyperbahn uses as=json for the advertise protocol so we cannot break hyperbahn.

I recommend that we do the following:

fix node so that arg2 is a map<string, string> encoded as JSON
fix python so that arg2 is a map<string, string> encoded as JSON

Raynos commented 8 years ago

Looks like @abhinav is working on updating python.

CLAassistant commented 5 years ago

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

abhinav commented 5 years ago

Issue was addressed.