{Compute} Update function of auto_shutdown_vm #17352
Conversation
|
@BigCat20196 An example PR. #17344 |
| if webhook: | ||
| if email: | ||
| if not webhook: | ||
| raise CLIError('usage error: --webhook is a required parameter') |
There was a problem hiding this comment.
When is it a required parameter? The error message could be better.
| 'emailRecipient': email, | ||
| 'webhookUrl': webhook, | ||
| 'timeInMinutes': 30, | ||
| 'status': 'Enabled' | ||
| } |
There was a problem hiding this comment.
Yeah, it's unnecessary. I took it out
BigCat20196
changed the title
yungezz
changed the title
| daily_recurrence = {'time': time} | ||
| notification_settings = None | ||
| if webhook: | ||
| if email: |
There was a problem hiding this comment.
is this service behavior? If no, this will be breaking change to end users: adding a required param for email.
There was a problem hiding this comment.
Yeah, the webhook parameter is also required if the user needs to set --email. This issue is that the user doesn't know the settings -- email needs to be set -- webhook. I make this change to prompt the user that both parameters must exist at the same time for them to take effect
There was a problem hiding this comment.
Do we allow webhook and no email?
| 'timeInMinutes': 30, | ||
| 'status': 'Enabled' | ||
| } | ||
| } |
3 participants
Description
Resolve #16824
[Compute] When users use --email but don't use --webhook, remind users that these two parameters need to be set together.
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.