Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flatten "properties" to avoid unnecessary nesting #1447

Closed
matthchr opened this issue Sep 28, 2020 · 4 comments · Fixed by #1548
Closed

Flatten "properties" to avoid unnecessary nesting #1447

matthchr opened this issue Sep 28, 2020 · 4 comments · Fixed by #1548

Comments

@matthchr
Copy link
Member

For example in the generated Redis types, there is a field Properties which probably ought to have its fields promoted up a level.

This is a pattern that is quite common (for example Storage account also has a Properties)

@matthchr matthchr changed the title We should consider detecting "Properties" fields in specs up a level to make for a cleaner spec. We should consider detecting "Properties" fields in specs up a level to make for a cleaner spec Sep 28, 2020
@matthchr matthchr changed the title We should consider detecting "Properties" fields in specs up a level to make for a cleaner spec We should consider promoting "Properties" fields in specs up a level to make for a cleaner spec Sep 29, 2020
@babbageclunk
Copy link
Member

I thought this quite often while going through the AnyType occurrences. Do you mean specifically for fields called "Properties"? Also, I'd guess we would want this to be recursive just in case there are multiple levels of Properties?

@matthchr
Copy link
Member Author

Yeah, specifically the properties field on the spec.

Not sure about nested properties. That first properties field is actually an artifact of ARM (everything outside of it is "for ARM", everything inside of it is "for the RP/Service") - it's a confusing distinction for actual consumers of the API though and AutoRest supports field collapsing to squash the contents of it up into the top level object. Removing that reduces nesting and I think all ARM resources have it.

Nested properties are a different story as those may actually be in the service API?

@Porges
Copy link
Member

Porges commented Sep 29, 2020

I think we can automatically detect what should be processed as some of the types are marked as flatten in the Swagger specs. (But this takes us further away from the ARM JSON…)

Here’s the documentation for x-ms-client-flatten.

@babbageclunk
Copy link
Member

Ah ok - definitely for the properties on a spec particularly.

@Porges Porges transferred this issue from Azure/k8s-infra May 14, 2021
@matthchr matthchr added this to the codegen-alpha-0 milestone May 18, 2021
@matthchr matthchr changed the title We should consider promoting "Properties" fields in specs up a level to make for a cleaner spec Flatten "properties" to avoid uncessecary nesting May 26, 2021
@matthchr matthchr changed the title Flatten "properties" to avoid uncessecary nesting Flatten "properties" to avoid unnecessary nesting May 26, 2021
@Porges Porges mentioned this issue Jun 2, 2021
2 tasks
Porges added a commit that referenced this issue Jun 27, 2021
Closes #1447

**What this PR does / why we need it**:

Applies the `x-ms-client-flatten` attribute when it is present in the Swagger.

**How does this PR make you feel**:
![gif](https://media.giphy.com/media/2a5IGQ1n1Ap1e/giphy.gif)
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 a pull request may close this issue.

3 participants