dapr / components-contrib

Community driven, reusable components for distributed apps
Apache License 2.0
548 stars 479 forks source link

metadata.yaml schema: improvements on default values and examples #3083

Open ItalyPaleAle opened 1 year ago

ItalyPaleAle commented 1 year ago

For our components' metadata, all values are technically strings: even numbers, are interpreted as strings in Component YAMLs.

In the metadata.yaml, we try to reflect this by using quotes around all examples and default values. Sometimes that makes us include awkward double quotes, such as:

https://github.com/dapr/components-contrib/blob/b3e2b1024a508d2aca07a124ea5a6cf63ff9a70c/bindings/http/metadata.yaml#L48-L49

This should be improved in two ways.

First: remove double quotes - let renderers add quotes when they render the metadat.ayaml

We should update all metadata.yaml documents to not include double quotes, so things like default: '"100Mi"' will become default: "100Mi" (or even just default: 100Mi when it's valid YAML)

Renderers should then add quotes around all examples and default values automatically

Second: update schema for examples

The change above is fine for the default value, but it does cause issues with examples since examples may contain multiple values (e.g. example:"true" or "false") where quotes do matter (we can't rely on renderers to add them automatically), or even explanations such asexample: '"100" (as bytes)`

To fix that, we can:

  1. Make example support arrays too. If a single string is passed, the renderer will quote it. If an array is passed, they are considered as multiple examples and they will be quoted independently in the renderer
  2. Add a field exampleRaw which is not quoted automatically and allows typing things such as exampleRaw: '"100" (as bytes)
    • example and exampleRaw are mutually-exclusive
berndverst commented 1 year ago

This might not go into 1.12 - we are technically beyond code freeze and while this isn't core code, it still requires reviewer time. Let's see when we get a PR for this and then we can decide. 1.13 is looking more likely.

artursouza commented 1 year ago

I am OK for this to be in 1.13, I don't want to add more to 1.12 unless absolutely necessary.

ItalyPaleAle commented 1 year ago

I put it in 1.12 to track it better as something aspirational. Doesn't need to be done in the next few weeks :) But, if it does happen, I'd happily review it. metadata.yaml files are sort-of "evergreen" anyways, as changes to those do not end up in the compiled binary.

github-actions[bot] commented 1 year ago

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.