Fix: removing params from GetServer #237
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Porges
added a commit
that referenced
this pull request
May 11, 2021
We need to transform the resources to include the “base types” that are added in the deployment template. This is hard to do when the handlers for `allOf` and `oneOf` do type mangling inside the JSON type extractor. So, create `AllOfType` and `OneOfType` to preserve these types until later and then “lower” them to `ObjectType` later in the pipeline. Next, greatly improve the way that `allOf` handling works by expanding the cases it works for. This is all under `intersectTypes` in the new pipeline file. This will fix #211. Changes made (in vaguely “pipeline order”): 1. Add an alternate entrypoint to `jsonast` (`GenerateDefinitionsFromDeploymentTemplate`) which knows specifically how to handle the deployment template schema and extract the resource types with their associated “base” types. 1. Promote `oneOf`/`allOf` to `Type`, and preserve them until later in the pipeline where they are converted to `ObjectType`. - `oneOf`/`allOf` have smart constructors which try to simplify the types a little (but not too much, they are handled fully later on) 1. Treat more things as resources in `jsonast`: 1. Definitions in `unknown_resourceDefinitions` as well as `resourceDefinitions` (EventGrid has resources here) 1. Any of the `*resourcebase*` types. This is needed to inherit “resourceness” for child resources (and maybe other embedded resources?) - This still feels a bit hacky, It was the simplest way to get child resources to be treated as resources but then the `*resourcebase*` “resources” need to be stripped out later. Any better ideas? 1. Add a pass to “lower” `anyOf`/`allOf` to `ObjectType` (`convertAllOfAndOneOfToObjects`). This handles many more cases for `allOf` to convert input schema to types that make more sense. We have good test coverage of this now. 1. Add a pass to “flatten” resources, which will take resources that have resources as spec/status and flatten that into a single resource. This is caused by some child resources and due to the new `resourcebase` modifications mentioned above. Extra: 1. Fix some output generation where types from other packages need qualification in ARM to/from conversions. 1. All `Type`s now implement `Stringer` to print something simple for debugging/logging purposes. 1. Remove struct-type check from resource factory function as the spec is no longer expected to be a struct (can be `oneOf`/`allOf`).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Fixing error
controllers/sqlserver_controller.go:58:15: undefined: instance controllers/sqlserver_controller.go:109:55: too many arguments in call to sdkClient.CheckNameAvailabilitySwitching
(string)to()Special notes for your reviewer:
If applicable: