Skip to content

Comments

Add directives for Python object deserialization#3343

Merged
lmazuel merged 2 commits intoAzure:masterfrom
alexeldeib:pyObject
Jul 6, 2018
Merged

Add directives for Python object deserialization#3343
lmazuel merged 2 commits intoAzure:masterfrom
alexeldeib:pyObject

Conversation

@alexeldeib
Copy link
Contributor

@alexeldeib alexeldeib commented Jul 2, 2018

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

@AutorestCI
Copy link

AutorestCI commented Jul 2, 2018

Automation for azure-sdk-for-python

A PR has been created for you:
Azure/azure-sdk-for-python#2862

@alexeldeib
Copy link
Contributor Author

The schema of "table" object in the response should be array of array of "any" value type, but is actually array of array of strings. AFAIK, autorest still doesn't have a way to do this that is universal to all clients. On a per language basis I've looked at how to address this. For Python we can model this as an "object" type, producing the desired wildcard behavior.

Also overriding the client name. It's outdated branding, see also my comment in #3269 (diff)

@lmazuel FYI

@AutorestCI
Copy link

AutorestCI commented Jul 2, 2018

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@AutorestCI
Copy link

AutorestCI commented Jul 3, 2018

Automation for azure-sdk-for-node

A PR has been created for you:
Azure/azure-sdk-for-node#3102

@AutorestCI
Copy link

AutorestCI commented Jul 3, 2018

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Jul 3, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@hovsepm hovsepm requested review from annatisch and lmazuel and removed request for lmazuel July 3, 2018 18:39
@hovsepm
Copy link
Contributor

hovsepm commented Jul 3, 2018

@lmazuel @annatisch could one of you take this PR? I have no idea about python directives.

@hovsepm hovsepm removed their request for review July 3, 2018 18:41
@hovsepm hovsepm assigned lmazuel and annatisch and unassigned hovsepm Jul 3, 2018
@lmazuel lmazuel requested review from lmazuel and removed request for annatisch July 3, 2018 19:09
@lmazuel
Copy link
Member

lmazuel commented Jul 3, 2018

@alexeldeib Sounds good to me!

@amarzavery @sarangan12 This is designed for "not strong types language" like Python and might interest you as well for NodeJS / Ruby. Maybe TS @RikkiGibson . This changes a model type from "string" to "object" using a transform rule (won't work on C#) to keep the value in the JSON "as-is" and don't stringify it.

@RikkiGibson
Copy link
Member

Ah, so does this transform make it so you don't just get a string of JSON-encoded data, but instead get the raw object tree returned by JSON.parse or the equivalent in the particular language?

@lmazuel
Copy link
Member

lmazuel commented Jul 3, 2018

@RikkiGibson correct. Because it can be integer, float, string, etc. exact types depends (I think of it as "excel cell content" analogy). Type is basic types enough so just showing the raw JSON value is enough.

@lmazuel lmazuel merged commit ac5f4d6 into Azure:master Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants