-
Notifications
You must be signed in to change notification settings - Fork 925
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
Proposal: Deprecate the usage of NoOptDefVal on flags #1442
Comments
@rikatz I've been looking at this, and agree to the deprecation of /assign @varshaprasad96 |
/triage accepted |
I've clearly been abusing this, as someone who introduced it for both dry-run and validation. I'm not sure HOW I would have done without it. |
@apelisse I don't think there was another way of doing it! But I think right now at least IMHO is a behavior that we should avoid, for command consistency, and make sure that if you want "--validation", you either call the flag a bool (validation=true,false) or a string (validation=type). Or, let's say, in the future if we say that dry-run is not a bool anymore, call the flag "dry-run-mode=client" (yikes, I know, it sucks, but I cannot also think on a different way) |
I just bumped into a highly undesired behavior performing a By omitting the equals sign, One thing that makes this even worse is that the bash auto-completion mechanism always sets Below is a snippet of commands with what's been described: $ kubectl delete sts my-sts --cascade orphan --dry-run server
W0701 19:49:02.551736 8944 helpers.go:663] --dry-run is deprecated and can be replaced with --dry-run=client.
statefulset.apps "my-sts" deleted (dry run)
statefulset.apps "orphan" deleted (dry run)
statefulset.apps "server" deleted (dry run)
$ kubectl delete sts my-sts --cascade orphan --dry-run=server
statefulset.apps "my-sts" deleted (server dry run)
Error from server (NotFound): statefulsets.apps "orphan" not found
$ kubectl delete sts my-sts --cascade=orphan --dry-run=server
statefulset.apps "my-sts" deleted (server dry run) Hope this will get sorted at some point. Thanks! |
@salavessa I consider this a bug in cobra. would you be willing to open an issue? spf13/cobra |
So, after thinking about it some more, I don’t think cobra can do much better. In your example @salavessa, if you trigger completion: Maybe not adding the space in this case would be slightly more informative? |
I think this is something that needs to be solved with user alerting, ie. the interactive delete flag that @ardaguclu is working on, coupled with the kuberc stuff I've been working in. That will force users to verify destructive actions and would catch something like this, assuming the user doesn't just pass a autoyes to everything. |
I'm not very concerned about the use of For What I would expect is that
For Thanks |
This PR introduces a proposal to address the issue kubernetes/kubectl#1442. It intends to: 1. Ensure consistency by requiring that all flags are accepted as key-value pairs. 2. Remove the use of `NoOptDefVal` from kubectl. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This PR introduces a proposal to address the issue kubernetes/kubectl#1442. It intends to: 1. Ensure consistency by requiring that all flags are accepted as key-value pairs. 2. Remove the use of `NoOptDefVal` from kubectl. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This PR introduces a proposal to address the issue kubernetes/kubectl#1442. It intends to: 1. Ensure consistency by requiring that all flags are accepted as key-value pairs. 2. Remove the use of `NoOptDefVal` from kubectl. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This PR introduces a proposal to address the issue kubernetes/kubectl#1442. It intends to: 1. Ensure consistency by requiring that all flags are accepted as key-value pairs. 2. Remove the use of `NoOptDefVal` from kubectl. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This PR introduces a proposal to address the issue kubernetes/kubectl#1442. It intends to: 1. Ensure consistency by requiring that all flags are accepted as key-value pairs. 2. Remove the use of `NoOptDefVal` from kubectl. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This issue has not been updated in over 1 year, and should be re-triaged. You can:
For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/ /remove-triage accepted |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
I did this in production and caused a (small internal) outage. In most CLI's I have used |
What would you like to be added:
Not added, but removed 😓
We use NoOptDefVal on flags like dry-run, validate and others (mapped below). This generates confusion on the command, as we accept string flags as
key=value
orkey value
, but when using NoOptDefVal this is not possible.This separation of a value makes sense, for instance, let's take
validate
flag as an example:In this case, do I want to create a deploy called
strict
and default value, or do I want to do strict validation but I forgot the deployment name? It is not possible to get it.We already stated that this behavior will be deprecated on the
dry-run
flag, but I think we should move forward and do with all the other string flags.If you pass a string flag, you should know what value you want (and not be susceptible to future behavior change if we want to change the "default hidden value".
This will also help to generate consistency in command usage, knowing that string flags can or can't be separated by "=" and that anything else is the real command arg.
Why is this needed:
Generate some consistency on command execution
I will try to remember the next sig-cli meeting to bring this subject, but also anyone looking at this feel free to comment here and tell me if it is worth to discussing this or if this behavior is this way and will not change, and close the issue :)
Commands and flags currently with this behavior
validate
anddry-run
flags oncreate *
commandcascade
flag ondelete *
commandset-raw-bytes
flag onconfig set
commandinsecure-skip-tls-verify
andembed-certs
flag onconfig set-cluster
commandembed-certs
flag onconfig set-credentials
commandThe text was updated successfully, but these errors were encountered: