Fix #8921: Command az keyvault key/secret/certificate list/list-deleted/list-versions with parameter --maxresults do not work as intended
#11531
Conversation
bim-msft
requested review from
arrownj,
fengzhou-msft,
jiasli,
qwordy and
yungezz
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| for scope in ['list', 'list-deleted', 'list-versions']: | ||
| with self.argument_context('keyvault key {}'.format(scope)) as c: | ||
| c.argument('maxresults', options_list=['--maxresults'], type=int) |
There was a problem hiding this comment.
is it necessary to set a upper limit on maxresults?
There was a problem hiding this comment.
@yungezz We have already set the lower bound and upper bound of maxresults in swagger and SDK code, it's not necessay to set it again in CLI code.
There was a problem hiding this comment.
What would happen if maxresults passed in is greater than 25?
There was a problem hiding this comment.
@fengzhou-msft CLI will throw an error message:
(env3) D:\repos\azure-cli>az keyvault secret list --vault-name bim-foo --maxresults 100
Parameter 'maxresults' must be less than 25.
There was a problem hiding this comment.
Ok. At least users know the limit from the error message. If you could put it in help text, that will be more clear, just a suggestion though.
There was a problem hiding this comment.
@fengzhou-msft Yes, I agree. I think it's a better idea that the upcoming CLI codegen system auto-generates such type of help messages from swagger file.
There was a problem hiding this comment.
Good to know that you will collect all these feedbacks and backpropagate to CodeGen team.
4 participants
Fix #8921
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.