Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The argument name in the deprecation warning message should use the format "--foo-bar" instead of "foo_bar" #24647

Closed
bingosummer opened this issue Nov 14, 2022 · 5 comments · Fixed by Azure/azure-cli-extensions#5550
Assignees
Labels
Auto-Assign Auto assign by bot Azure CLI Team The command of the issue is owned by Azure CLI team Knack question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@bingosummer
Copy link
Member

Related command

$ az aks create -g binxi1114 -n binxi111401 --enable-pod-security-policy -c 1
Argument 'enable_pod_security_policy' has been deprecated and will be removed in a future release.
Argument '--enable-pod-security-policy' is in preview and under development.

Extension name (the extension in question)

It's not an issue to a specific extension. It is a common issue.

Description of issue (in as much detail as possible)

I'm working a PR to deprecate the argument '--enable-pod-security-policy', and I get the message like below. The warning message should be "Argument '--enable-pod-security-policy' has been deprecated and will be removed in a future release.". Argument 'enable_pod_security_policy' is not good.

$ az aks create -g binxi1114 -n binxi111401 --enable-pod-security-policy -c 1
Argument 'enable_pod_security_policy' has been deprecated and will be removed in a future release.
Argument '--enable-pod-security-policy' is in preview and under development.

@ghost ghost added AKS az aks/acs/openshift CXP Attention This issue is handled by CXP team. Auto-Assign Auto assign by bot labels Nov 14, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Nov 14, 2022

route to CXP team

@PramodValavala-MSFT PramodValavala-MSFT self-assigned this Nov 15, 2022
@PramodValavala-MSFT PramodValavala-MSFT added the question The issue doesn't require a change to the product in order to be resolved. Most issues start as that label Nov 15, 2022
@PramodValavala-MSFT
Copy link
Contributor

@bingosummer Thanks for bringing this up! Looks like that is how it is handled in the knack framework (this code here) but looks like if you use the options_list parameter when defining the argument, it would be as expected.

Nonetheless, I believe showing the argument is not best for customers since that would just be confusing. @yonzhan @jiasli Could you share insights on why the default behavior is so?

@bingosummer
Copy link
Member Author

seems options_list doesn't help in my case.

@jiasli jiasli transferred this issue from Azure/azure-cli-extensions Nov 15, 2022
@jiasli
Copy link
Member

jiasli commented Nov 15, 2022

It is the designed behavior of knack to show argument_dest in the warning message if deprecation target is not specified:

https://github.com/microsoft/knack/blob/cd590312b9b4ea1402d7a0a76814f6ca4da7fab8/knack/arguments.py#L221-L222

        if deprecate_info:
            deprecate_info.target = deprecate_info.target or argument_dest

To override the enable_pod_security_policy in the warning message, please specify

deprecate_info=c.deprecate(target='--enable-pod-security-policy')

Here is an example from resource command module:

c.argument('handle_extended_json_format', arg_type=extended_json_format_type,
deprecate_info=c.deprecate(target='--handle-extended-json-format/-j'))

Another example from vm command module:

c.argument('for_upload', arg_type=get_three_state_flag(), min_api='2018-09-30',
deprecate_info=c.deprecate(target='--for-upload', redirect='--upload-type Upload', hide=True),

@jiasli jiasli added Knack and removed AKS az aks/acs/openshift labels Nov 15, 2022
@bingosummer
Copy link
Member Author

deprecate_info=c.deprecate(target='--enable-pod-security-policy') works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot Azure CLI Team The command of the issue is owned by Azure CLI team Knack question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
4 participants