yarpc / yab

Call and benchmark YARPC services from the command line.
MIT License
86 stars 35 forks source link

Change input-decoder interface to return YAML request body #321

Closed jronak closed 3 years ago

jronak commented 3 years ago

Prior to this change input decoder exposed method which returns JSON body, but existing Serializer interface already exposes methods with YAML body. This change upholds the consistency between all the interfaces and also avoids the lossy conversion from YAML to JSON type.

Fixes https://github.com/yarpc/yab/pull/314#discussion_r565766802

codecov[bot] commented 3 years ago

Codecov Report

Merging #321 (99bd1d3) into dev (2a495ad) will increase coverage by 0.06%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #321      +/-   ##
==========================================
+ Coverage   90.27%   90.33%   +0.06%     
==========================================
  Files          52       52              
  Lines        2590     2587       -3     
==========================================
- Hits         2338     2337       -1     
+ Misses        178      177       -1     
+ Partials       74       73       -1     
Impacted Files Coverage Δ
encoding/inputdecoder/decoder.go 100.00% <100.00%> (+8.69%) :arrow_up:
encoding/protobuf.go 92.72% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2a495ad...6f3bbdc. Read the comment docs.

prashantv commented 3 years ago

Does this support multiple YAML documents? E.g.,

---
bar: 1
---
bar: 2

https://www.ribbybibby.me/posts/multiple-yaml-documents/

jronak commented 3 years ago

Does this support multiple YAML documents? E.g.,

---
bar: 1
---
bar: 2

https://www.ribbybibby.me/posts/multiple-yaml-documents/

Yup, --- is used as a delimiter by yaml decoder and it can parse multiple yaml docs from the same input reader.

Linking to a gRPC stream integration test which uses multiple YAML documents - https://github.com/yarpc/yab/blob/dace80fcc1439a7a161713a4f7b801ec738d702d/integration_test.go#L493-L500