search

fullstorydev / grpchan

Channels for gRPC: custom transports
MIT License
204 stars 23 forks source link

grpchan should support json-over-http #57

Closed dragonsinth closed 3 years ago

dragonsinth commented 3 years ago

Re-opening the original formulation of #40

dragonsinth commented 3 years ago

Obviously, I'm going to have to rebase this and make it work post encoder refactor. The big open question is, do we: 1) hard code the codecs as in this PR or 2) encoding.GetCodec("json"), but only for application/json?

dragonsinth commented 3 years ago

FWIW, if we wanted to support some variant of json streaming, we could probably use https://en.wikipedia.org/wiki/JSON_streaming#Record_separator-delimited_JSON

e.g.

const RecordSep = 0x1E
const Newline = 0x0a
jhump commented 3 years ago
  1. hard code the codecs as in this PR or
  2. encoding.GetCodec("json"), but only for application/json?

I'm personally for option one, since there's no on-rails way to wire up a JSON codec -- every consumer would have to implement it and register it.

Perhaps some middle ground: query for a codec, but if it doesn't exist use a default. And I think the default should probably be a zero-value jsonpb marshaler.

RE: streaming JSON, with Go it's easiest to do concatenated JSON (since json.Decoder knows how to parse one value at a time). But it would be more important to know what's easiest to do from the browser/TS side of things. I'd totally have streams fail fast and leave it as a TODO for now though...

dragonsinth commented 3 years ago

Leaving this branch in place to pin the SHA, but closing the PR. Superceded by #58