Skip to content

Support a new parameter called -ActionGroupId in the Add-AzMetricAlertRuleV2 command in Az.Monitor#11045

Merged
VeryEarly merged 5 commits intoAzure:masterfrom
sivanguetta:master
Feb 10, 2020
Merged

Support a new parameter called -ActionGroupId in the Add-AzMetricAlertRuleV2 command in Az.Monitor#11045
VeryEarly merged 5 commits intoAzure:masterfrom
sivanguetta:master

Conversation

@sivanguetta
Copy link
Copy Markdown
Contributor

@sivanguetta sivanguetta commented Feb 6, 2020

Description

Support a new parameter called -ActionGroupId in the Add-AzMetricAlertRuleV2 command in Az.Monitor.

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)
    • the markdown help files have been regenerated using the commands listed here

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 7, 2020

if I run:

Add-AzMetricAlertRuleV2 -Name $Name -ResourceGroupName $rg -WindowSize $Size -Frequency $Frequency -TargetResourceScope $ResourceScope -TargetResourceType $ResourceType -TargetResourceRegion $ResourceRegion -Condition $Condition -Severity $Severity

without providing "ActionGroup" or "ActionGroupId",

How powershell tell which parameter set to use?
Can you try test this scenario?

@sivanguetta
Copy link
Copy Markdown
Contributor Author

sivanguetta commented Feb 9, 2020 •
edited
Loading

if I run:

Add-AzMetricAlertRuleV2 -Name $Name -ResourceGroupName $rg -WindowSize $Size -Frequency $Frequency -TargetResourceScope $ResourceScope -TargetResourceType $ResourceType -TargetResourceRegion $ResourceRegion -Condition $Condition -Severity $Severity

without providing "ActionGroup" or "ActionGroupId",

How powershell tell which parameter set to use?
Can you try test this scenario?

@VeryEarly
Thanks, good catch.
I added 2 parameters sets CreateAlertByResourceId and CreateAlertByScopes, these parameters set will cover the cases that action group was not provided.
In addition I added scenario tests on this case.

BTW, are you familiar with a better practice to resolve this parameters combinations?

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 10, 2020

if I run:

Add-AzMetricAlertRuleV2 -Name $Name -ResourceGroupName $rg -WindowSize $Size -Frequency $Frequency -TargetResourceScope $ResourceScope -TargetResourceType $ResourceType -TargetResourceRegion $ResourceRegion -Condition $Condition -Severity $Severity

without providing "ActionGroup" or "ActionGroupId",
How powershell tell which parameter set to use?
Can you try test this scenario?

@VeryEarly
Thanks, good catch.
I added 2 parameters sets CreateAlertByResourceId and CreateAlertByScopes, these parameters set will cover the cases that action group was not provided.
In addition I added scenario tests on this case.

BTW, are you familiar with a better practice to resolve this parameters combinations?

I think what you did is recommended.

There are static analysis messages you need to suppress (because you used variables for parameter set names which were String before)

Also, for the new parameter sets you added, they should be in the help markdown file.

VeryEarly
VeryEarly requested changes Feb 10, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@VeryEarly VeryEarly left a comment

Choose a reason for hiding this comment

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

please also update the help markdown file for new added parameter sets

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 10, 2020

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented Feb 10, 2020

Azure Pipelines successfully started running 3 pipeline(s).

@sivanguetta
Copy link
Copy Markdown
Contributor Author

sivanguetta commented Feb 10, 2020

please also update the help markdown file for new added parameter sets

Done

@sivanguetta sivanguetta reopened this Feb 10, 2020
@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 10, 2020 •
edited
Loading

/azp run azure-powershell - powershell-core

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented Feb 10, 2020

No pipelines are associated with this pull request.

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented Feb 10, 2020

Azure Pipelines successfully started running 1 pipeline(s).

@sivanguetta
Copy link
Copy Markdown
Contributor Author

sivanguetta commented Feb 10, 2020

/azp run azure-powershell - powershell-core

@VeryEarly
This is my first PR on this solution, I'm not familiar with this command.
Can you please elaborate or refer me to documentation about it?

@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 10, 2020 •
edited
Loading

/azp run azure-powershell - powershell-core

@VeryEarly
This is my first PR on this solution, I'm not familiar with this command.
Can you please elaborate or refer me to documentation about it?

sorry for the confusion, this command is for triggering the CI job "azure-powershell - powershell-core"

@sivanguetta
Copy link
Copy Markdown
Contributor Author

sivanguetta commented Feb 10, 2020

/azp run azure-powershell - powershell-core

@VeryEarly
This is my first PR on this solution, I'm not familiar with this command.
Can you please elaborate or refer me to documentation about it?

sorry for the confusion, this command is for triggering the CI job "azure-powershell - powershell-core"

How can I run it? I tried to run it from PowerShell and PS-VSPrompt but it didn't work?

@sivanguetta
Copy link
Copy Markdown
Contributor Author

sivanguetta commented Feb 10, 2020

@VeryEarly
Thank you for the review:-)
I see that all checks have passed but I can't merge the pull request because I'm not authorized.
How can I merge the PR? what is missing?

@sivanguetta sivanguetta requested a review from adibiton February 10, 2020 16:09
@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 10, 2020

/azp run azure-powershell - powershell-core

@VeryEarly
This is my first PR on this solution, I'm not familiar with this command.
Can you please elaborate or refer me to documentation about it?

sorry for the confusion, this command is for triggering the CI job "azure-powershell - powershell-core"

How can I run it? I tried to run it from PowerShell and PS-VSPrompt but it didn't work?

You can run it on github like I did.

@VeryEarly VeryEarly merged commit b10da39 into Azure:master Feb 10, 2020
@VeryEarly
Copy link
Copy Markdown
Collaborator

VeryEarly commented Feb 10, 2020 •
edited
Loading

@VeryEarly
Thank you for the review:-)
I see that all checks have passed but I can't merge the pull request because I'm not authorized.
How can I merge the PR? what is missing?

Once your PR was passed all checks and was approved, member of azure-powershell team will merge it for you.

dingmeng-xue pushed a commit to dingmeng-xue/azure-powershell that referenced this pull request Mar 29, 2020
@VeryEarly
Merge pull request Azure#11045 from sivanguetta/master
9a9696f 
Support a new parameter called -ActionGroupId in the Add-AzMetricAlertRuleV2 command in Az.Monitor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VeryEarly VeryEarly VeryEarly approved these changes

@meday89 meday89 Awaiting requested review from meday89

@markcowl markcowl Awaiting requested review from markcowl

@isra-fel isra-fel Awaiting requested review from isra-fel

@adibiton adibiton Awaiting requested review from adibiton

Assignees

@sivanguetta sivanguetta

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sivanguetta @VeryEarly