First version of Azure NetApp Files CLI for GA #9365
Conversation
Previously the Azure NetApp Files CLI was an extension only. R4 standard of RP for GA
NFSAAS-2316 first version of ANF CLI
Previously the Azure NetApp Files CLI was an extension only. R4 standard of RP for GA
NFSAAS-2316 first version of ANF CLI
|
@tjprescott There are CI failures but it seems I should be merging branches dev to dev not master to master. Is that correct? |
|
No one is allowed to make PRs to the master branch. |
tjprescott
left a comment
tjprescott
left a comment
There was a problem hiding this comment.
This will require a full command review. Please schedule accordingly.
| ++++++++++++++++++ | ||
|
|
||
| * GA release. | ||
| * Previously existed as cli-extension. |
There was a problem hiding this comment.
You will need to schedule a full command review to move this from an extension to a module. Do not put the date in the version. "Previously existed as cli-extension" is not a customer-facing thing.
There was a problem hiding this comment.
You will need to schedule a full command review to move this from an extension to a module. Do not put the date in the version. "Previously existed as cli-extension" is not a customer-facing thing.
Ok. I will remove the date and the cli-extension text. Can you tell me the process for the command review? I know one of these was already performed, initiated through email with Shane Mainali among others, and the commands agreed in a meeting. It was then decided that because the swagger was in preview that the service should be an extension. I did not expect another review was needed. Actually I also used the information here https://github.com/Azure/azure-cli/blob/dev/doc/extensions/faq.md#how-do-i-move-from-an-extension-to-a-command-module as a guide which doesn't mention the need for a further review either.
| netappfiles_custom = CliCommandType(operations_tmpl='azure.cli.command_modules.netappfiles.custom#{}') | ||
| super(NetAppFilesCommandsLoader, self).__init__(cli_ctx=cli_ctx, | ||
| min_profile='2017-03-10-profile', | ||
| custom_command_type=netappfiles_custom) |
There was a problem hiding this comment.
You need to add logic to suppress the extension here or the older extension will override your newer module.
There was a problem hiding this comment.
You need to add logic to suppress the extension here or the older extension will override your newer module.
Ok that makes good sense. What needs added to perform this?
|
|
||
|
|
||
| def netappfiles_exception_handler(ex): | ||
| if isinstance(ex, CloudError) or isinstance(ex, ValidationError) or isinstance(ex, ValueError): |
There was a problem hiding this comment.
This seems very generic. The default CLI exception handler should be sufficient.
There was a problem hiding this comment.
This seems very generic. The default CLI exception handler should be sufficient.
Ok. What is the default CLI exception? Should I not capture the CloudError? The ValidationError and ValueError were added deliberately because otherwise certain invalid commands would result in a trace log which didn't seem to be what a CLI user would want to see. Please advise further.
| short-summary: A list of space separated tags to apply to the account | ||
| - name: --active-directories | ||
| type: string | ||
| short-summary: An array of active directory (AD) settings in json format. Limitation one AD/subscription. Consists of the fields username (Username of Active Directory domain administrator), password (Plain text password of Active Directory domain administrator), domain (Name of the Active Directory domain), dns (Comma separated list of DNS server IP addresses for the Active Directory domain), smb_server_name (NetBIOS name of the SMB server. This name will be registered as a computer account in the AD and used to mount volumes. Must be 10 characters or less), organizational_unit (The Organizational Unit (OU) within the Windows Active Directory) |
There was a problem hiding this comment.
For this alone, this module can only be created in preview.
There was a problem hiding this comment.
For this alone, this module can only be created in preview.
Sorry I don't really get your meaning here. Can you explain a bit further?
Ok. And this is the case even though it had a command review for the extension? Can you tell me how I go about this? Thanks. |
Ok. So what branch should I be using? Thanks. |
|
@tjprescott Please give me some feedback on these comments. In particular if a new command review is required then I need to know how and get this moving as soon as I can. Thanks. |
|
Your PR needs to target the dev branch. A full review is required when a new module/extension is created, when transitioning from extension to module and when declaring a module GA. |
|
@tjprescott I'm closing this PR and opening #9387 on dev branch. I will also initiate a command review but as yet have no info on whether there is a formal process (as for PowerShell) so will just be including some Microsoft and NetApp folks in an email thread as was done previously. However you have raised some points here which look important to include in the code change and as yet I've had no feedback on my questions. Thanks in advance. |
2 participants
Previously the Azure NetApp Files CLI was an extension only.
R4 standard of RP for GA
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.