cli icon indicating copy to clipboard operation
cli copied to clipboard

Change `dapr annotate` command to use a `--set` based model

Open mukundansundar opened this issue 2 years ago • 3 comments

Describe the proposal

As it exists, in dapr annotate command, we have a flag defined for each annotation. That in itself has defined a huge number of flags for that particular command also there are slight variations between the flag in dapr annotate command the one annotations themselves. For eg: for most of the sidecar based annotations, the sidecar prefix is left off like cpu-limit or cpu-request. Additionally, as the annotations grow, the number of flags need to grow as well and we need to keep track of each and every annotation that is added to make sure that it is added as part of the dapr annotate command so that there is no divergence.

The proposal here is to have a --set flag which gets input as a string which is directly set as the annotation for the sidecar, eg: --set dapr.io/sidecar-cpu-limit: 1. This way we use a single flag for any number of annotations.

@yaron2 @jjcollinge Thoughts on this?

Release Note

RELEASE NOTE: Changed dapr annotate command to have a single --set flag

mukundansundar avatar Jul 18 '22 09:07 mukundansundar

thanks @mukundansundar this proposed change sounds good to me if we assume dapr annotate would only really be used by users who are familiar with the existing flags/settings - we should probably link to the docs page in the help message so people can look up the keys easily (we could the annotation list directly in the CLI too). I think it might also make sense to have some basic key validation to help the users set the dapr annotations correctly rather than this just setting any string they pass. Keeping this validation logic in sync with the other flags is much less work as it'd just be adding the keys to a slice or map. We could also make the dapr.io prefix optional to make the command a little less verbose. Wdyt?

jjcollinge avatar Jul 18 '22 09:07 jjcollinge

I am all for adding validation and if we can keep it in sync with little less work then we should go for it. Though I am wondering if dapr.io needs to made optional ? Will there be a use case where the user will use this command to automate setting of other annotations other than dapr ones and in that case how will we identify dapr specific annotations ?

mukundansundar avatar Jul 18 '22 13:07 mukundansundar

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

dapr-bot avatar Aug 22 '22 03:08 dapr-bot