search

Azure / msrest-for-python

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

Fix #11 - Allow polymorphic serialization from JSON like objects #12

Closed lmazuel closed 7 years ago

lmazuel commented 7 years ago

Didn't test with Autorest testsuite yet, but since this is JSON-like syntax, I'm pretty sure there is no test in Autorest anyway.

lmazuel commented 7 years ago

Tested by @vishrutshah on his Monitor issue and all SDK tests ok.

lmazuel commented 7 years ago

@annatisch I dig inside the msrest code and I realize I re-wrote _classify but calling it _infer_type. I will rework on this to simplify code and avoid duplication.

annatisch commented 7 years ago

@lmazuel - yes I thought there might be some duplication there - I was about to compare them myself :)

lmazuel commented 7 years ago

@annatisch if you have the time, I pushed an update that should still fix the serialization + the deserialisation at the time (rewriting classify to include both fix). I didn't launch Autorest tests or SDKs yet.

lmazuel commented 7 years ago

@annatisch I think everything is here and fix all these issues:

Thoughts?

annatisch commented 7 years ago

@lmazuel - yup looks like it! :) Yay!

lmazuel commented 7 years ago

@annatisch last commit was debugged directly with @vishrutshah in peer programming. This allows to accept as key in the dict (serialisation) both the JSON key name and the Python key name. Let's say RestAPI uses "awesomeName", so Python will use "awesome_name". Now for subtyping, both syntax are correct in a dict. This allows to figure out subtyping even from a dict (serialisation) and from a JSON (deserialisation) even if the key are not expected to be the same. Also, it's been a long time I think about accepting both keys syntax in dict serialization (to simplify a copy/paste from a RestAPI example), so I'm shocked and don't consider this a hack. Unittest to be added before merging this PR.