Skip to content

Conversation

@akshayneema
Copy link
Contributor

Description
Added checks wherever KeyError was happening and raised customized error messages according to the situation. Did some improvements in the help text for a clearer understanding of the user.

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.


This checklist is used to make sure that common guidelines for a pull request are followed.

@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Oct 29, 2020
@ghost
Copy link

ghost commented Oct 29, 2020

Thank you for your contribution akshayneema! We will review the pull request and get back to you soon.

@yungezz
Copy link
Member

yungezz commented Oct 30, 2020

hi @fengzhou-msft could you help to review?


protectable_item_type_map = {'SQLDatabase': 'SQLDataBase',
'HANADataBase': 'SAPHanaDatabase',
'SAPHanaDatabase': 'SAPHanaDatabase',
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a breaking change. If someone has used HANADataBase as protectable item type in their script, it will start failing. Better to keep the older one and add another entry to the map

return AzureVmWorkloadSQLDatabaseProtectedItem()


def _check_map(key, key_value_map):
Copy link
Contributor

Choose a reason for hiding this comment

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

Use better naming conventions.

@fengzhou-msft fengzhou-msft requested a review from houk-ms November 2, 2020 05:14
if item_type_map.get(item_type) is not None:
return item_type_map[item_type]
error_text = item_type + " is an invalid argument. " + str(list(item_type_map.keys())) + " are the allowed values."
raise CLIError(error_text)
Copy link
Member

Choose a reason for hiding this comment

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

Please use InvalidArgumentValueError for better error classification. @houk-ms for awareness.

@fengzhou-msft fengzhou-msft merged commit b64a1e8 into Azure:dev Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

customer-reported Issues that are reported by GitHub users external to the Azure organization. Recovery Services Backup az backup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants