Azure / msrest-for-python

The runtime library "msrest" for AutoRest generated Python clients.
MIT License
41 stars 64 forks source link

serialization: avoid empty additionalProperties #89

Closed yugangw-msft closed 6 years ago

yugangw-msft commented 6 years ago

For https://github.com/Azure/azure-cli/issues/5733 I thought more about pruning at CLI level, but it has perf related drawback of having to go recursively to the leaf level; also, commands can create its own dictionary typed output which includes model instances, so CLI can't just check the root level types. So submit this PR to get conversation going. I am fine with any other options as long as we can mitigate the confusion. Json output is supposed to honestly reflect what is on the wire, and i hope the spirit can continue. //CC: @lmazuel @tjprescott @derekbekoe

codecov-io commented 6 years ago

Codecov Report

Merging #89 into master will increase coverage by 0.15%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   28.94%   29.09%   +0.15%     
==========================================
  Files         380      380              
  Lines       16332    16170     -162     
==========================================
- Hits         4727     4705      -22     
+ Misses      11605    11465     -140
Impacted Files Coverage Δ
msrest/serialization.py 82.29% <100%> (-0.03%) :arrow_down:
test/vanilla/AcceptanceTests/test_url.py 22.72% <0%> (+0.17%) :arrow_up:
...t/vanilla/AcceptanceTests/test_model_flattening.py 21.66% <0%> (+1.29%) :arrow_up:
test/vanilla/AcceptanceTests/test_dictionary.py 13.04% <0%> (+1.88%) :arrow_up:
.../vanilla/AcceptanceTests/test_required_optional.py 31.25% <0%> (+3.97%) :arrow_up:

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 9d81113...bd892c5. Read the comment docs.

yugangw-msft commented 6 years ago

@lmazuel, i am closing this out since you are not convinced this is the right thing to do