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

GET VMSS expand parameter not required #15602

Closed
fitzgeraldsteele opened this issue Sep 21, 2021 · 3 comments
Closed

GET VMSS expand parameter not required #15602

fitzgeraldsteele opened this issue Sep 21, 2021 · 3 comments
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Compute Mgmt This issue is related to a management-plane library.

Comments

@fitzgeraldsteele
Copy link

Bug Report

  • import path of package in question, e.g. .../services/compute/mgmt/2018-06-01/compute
    /services/compute/mgmt/2021-07-01/compute/virtualmachinescalesets.go#L553

  • SDK version e.g. master, latest, 18.1.0
    latest
    Release v57.0.0 1630045276 (Release v57.0.0 1630045276 #15400)

    • Specify the exact commit if possible; one way to get this is the REVISION
      column output by dep status "github.com/Azure/azure-sdk-for-go.
  • output of go version

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 21, 2021
@RickWinter RickWinter added bug This issue requires a change to an existing behavior in the product in order to be resolved. Compute Mgmt This issue is related to a management-plane library. labels Sep 21, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 21, 2021
@RickWinter RickWinter added this to the [2021] November milestone Sep 21, 2021
@fitzgeraldsteele
Copy link
Author

@RickWinter @lirenhe is there any way to get an RCA for this one? How is possible that an optional parameter in the API gets made as required in the SDK? Is there something we need to do differently in swagger?

@ArcturusZhang
Copy link
Member

Hi @fitzgeraldsteele the expand is not required in the SDK, but due to some bad design in our track 1 SDK, any optional string-like parameters are generated using the type of string because it can be empty instead of using nil to represent it is empty.
We know this is not a good design, but changing this would cause massive breaking changes, therefore we will not change that in track 1 SDK (the SDKs under services directory).
If you are interested, please take a look at the track 2 SDK (the SDKs under sdk directory):

func (client *VirtualMachineScaleSetsClient) Get(ctx context.Context, resourceGroupName string, vmScaleSetName string, options *VirtualMachineScaleSetsGetOptions) (VirtualMachineScaleSetsGetResponse, error) {

The query parameters will not "act" as if they are required like in track 1 SDKs

Since we are not able to change this in the SDK, I am going to close this issue.

@ArcturusZhang
Copy link
Member

And BTW, in track 1 SDK, you will have to assign an empty string to it if you do not want to pass expand parameters.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Compute Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

No branches or pull requests

4 participants