Skip to content

Updated Set-AzExpressRouteGateway to allow either one of -MinScaleUnits and -MaxScaleUnits#14145

Closed
nimaller wants to merge 1 commit intoAzure:masterfrom
nimaller:updateExpressRouteGatewayFix
Closed

Updated Set-AzExpressRouteGateway to allow either one of -MinScaleUnits and -MaxScaleUnits#14145
nimaller wants to merge 1 commit intoAzure:masterfrom
nimaller:updateExpressRouteGatewayFix

Conversation

@nimaller
Copy link
Copy Markdown
Contributor

@nimaller nimaller commented Feb 5, 2021

Description

Currently, when using set-azexpressroutegateway, users can specify min scale units or max scale units, and the request will succeed but the values will be ignored. This PR fixes it by validating the range in min and max scale units, and allowing only one of them to be specified.

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
      • {Please put the link here}
    • the markdown help files have been regenerated using the commands listed here

@nimaller nimaller changed the title initial changes Updated Set-AzExpressRouteGateway to allow either one of -MinScaleUnits and -MaxScaleUnits Feb 5, 2021
@VeryEarly VeryEarly self-assigned this Feb 7, 2021
[Parameter(
Mandatory = false,
HelpMessage = "Min for the scale units for this ExpressRouteGateway.")]
[ValidateRange(1, 100)]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a breaking change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are already validating bounds in NRP, so this will just fail the call earlier.

}

if (this.MinScaleUnits > 0 && this.MaxScaleUnits > 0)
if (this.MinScaleUnits > 0 || this.MaxScaleUnits > 0)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if one of these parameters was not present, can you please confirm whether line 128/129 still work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will set the parameter to 0, which is the expected behavior.

@VeryEarly
Copy link
Copy Markdown
Collaborator

please target your PR against network-november branch

@wyunchi-ms
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@wyunchi-ms
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@nimaller
Copy link
Copy Markdown
Contributor Author

nimaller commented Feb 7, 2021

New PR: #14150

@nimaller nimaller closed this Feb 7, 2021
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.

3 participants