Add Subscription RP commands #5453
Conversation
|
View a preview at https://prompt.ws/r/Azure/azure-cli/5453 |
|
|
||
| @Completer | ||
| def get_offer_type_completion_list(cmd, prefix, namespace): # pylint: disable=unused-argument | ||
| return ['MS-AZR-0017P', 'MS-AZR-0148P'] |
There was a problem hiding this comment.
A completer is overkill for this. They are only used when you need to query the server for completions. Instead use: arg_type=get_enum_typ(['MS-AZR-0017P', 'MS-AZR-0148P']) in your params.py file for this parameter.
| from knack.help_files import helps | ||
|
|
||
|
|
||
| helps['subscriptiondefinition'] = """ |
There was a problem hiding this comment.
This is a very lengthy name for a top-level command. Also, the account subgroup is already designated for subscriptions, so it would be more logical for these command to live there. az account definition ....
There was a problem hiding this comment.
Moved it under the account subgroup. Agree the name is long, but unfortunately I believe "definition" is too generic. Is there precedent for using shortened names, such as "subdef", so it becomes "account subdef list"?
| # pylint: disable=line-too-long | ||
| def load_arguments(self, _): | ||
| with self.argument_context('subscriptiondefinition show') as c: | ||
| c.argument('subscription_definition_name', options_list=['--name', '-n'], required=False, help='Name of the subscription definition to show.') |
There was a problem hiding this comment.
You don't need to call out "...to show". That way you only need to apply this alias once, with a scope of account definition.
|
|
||
| with self.argument_context('subscriptiondefinition create') as c: | ||
| c.argument('name', options_list=['--name', '-n'], required=True, help='Name of the subscription definition.') | ||
| c.argument('offer_type', options_list=['--offer_type', '-ot'], required=True, help='The subscription\'s offer type.', completer=get_offer_type_completion_list) |
There was a problem hiding this comment.
You cannot have a two-character short option. If anything there shouldn't be a short option, but if you really want one, -t would be the most appropriate.
You cannot use underscore in you long options. --offer_type should be --offer-type.
Just use arg_type=get_enum_type(...) here instead of a completer.
Finally, you don't need/should not provide required=True/False. The CLI will infer that from your command signature based on whether you provided a default value.
| with self.argument_context('subscriptiondefinition create') as c: | ||
| c.argument('name', options_list=['--name', '-n'], required=True, help='Name of the subscription definition.') | ||
| c.argument('offer_type', options_list=['--offer_type', '-ot'], required=True, help='The subscription\'s offer type.', completer=get_offer_type_completion_list) | ||
| c.argument('subscription_display_name', options_list=['--subscription_display_name', '-sdn'], required=False, help='The subscription display name of the subscription definition.') |
There was a problem hiding this comment.
Same comments as above. Also, I would recommend shortening --subscription-display-name to simply --display-name
There was a problem hiding this comment.
Fixed. Regarding subscription-display-name - just "display-name" doesn't properly capture the semantic here, so I'm leaving that as-is.
|
|
||
|
|
||
| def cli_subscription_create_subscription_definition(client, name, offer_type, subscription_display_name=None): | ||
| """Create a subscription definition.""" |
There was a problem hiding this comment.
Don't use custom command doc strings to populate help. Instead, add a command entry to you help.py file with this as the short-summary. The reason is because the live docs edit functionality will link directly to that, and will never find it here.
azure-cli2017.pyproj
Outdated
| @@ -14,7 +14,7 @@ | |||
| <LaunchProvider>Standard Python launcher</LaunchProvider> | |||
| <InterpreterId>MSBuild|{54f4b6dc-0859-46dc-99bb-b275c9d0aca3}|$(MSBuildProjectFullPath)</InterpreterId> | |||
| <EnableNativeCodeDebugging>False</EnableNativeCodeDebugging> | |||
| <CommandLineArguments></CommandLineArguments> | |||
| <CommandLineArguments>interactive</CommandLineArguments> | |||
|
Replacing this PR with Azure/azure-cli-extensions#49. |
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Command Guidelines
(see Authoring Command Modules)