-
Notifications
You must be signed in to change notification settings - Fork 243
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
Search for discriminator children through multiple levels of allOf nesting #1460
Search for discriminator children through multiple levels of allOf nesting #1460
Conversation
I'm starting to think it should be a breadth-first search rather than depth-first (what I've currently implemented). That way, we'd stop at the level where we find another "discriminator"... |
Thanks @krishanjmistry! I ran the code generation. Here is how I ran it: ~/ms/azure-rest-api-specs> git checkout 7c906b15744aaf7b782dfb82ccefcbef05cbf20b
~/ms/azure-sdk-for-rust/services/autorust> cargo run -p azure-autorust --release It fixed 7 control plane and 3 data place services. Once a unit test is added, this should be good to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again! For extra credit, a unit test that used to fail deserialization, but now works would be nice. json from a specification example could probably be copied.
I've added a test in the |
|
||
let item = list.value.first().expect("There should be at least one item in the list"); | ||
let properties= item.properties.as_ref().expect("The properties should be present"); | ||
assert!(matches!(properties, &WorkloadProtectableItemUnion::MicrosoftClassicComputeVirtualMachines(_))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. Thanks!
Attempts to fix #1458 - see the issue for more details.
I've tested this by modifying the
codegen/examples/resources_mgmt.rs
to point to:azure-rest-api-specs/specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2023-04-01/bms.json
This now produces the enum like so:
I'm unsure how to test this more generally across all the API's - happy to be steered on this