-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Enum Parameter Type #2610
Comments
@skaegi probably has some thoughts here, he's been working on adding something like a "schema" to params to allow for many different types beyond just the array and string we currently have |
/kind feature |
I'll add this to the agenda of our API working group on Monday to get some more eyes on it and see where the schema work (#1393) is at. |
For validation of params it's a bit of a balancing act as we want to retain backwards-compatibility at the same time as improving validation. For the validation in particular I'm hesitant to re-invent syntax here so what I'm thinking of borrows a lot from json schema. (See https://tools.ietf.org/html/draft-handrews-json-schema-validation-02#section-6.1.2) For enum I'd suggest something like...
(e.g. leave all schema aspects in a
(e.g. add peer schema aspects directly to the param [might be hard to support long term]) |
@andrewballantyne there is still a ton of work to actually support multiple types as per #1393 however we could still make progress on the validation aspect independently. e.g. |
This is definitely more open-ended and can evolve in the future to support anything. I think this is probably the better of the two solutions (and for the support of #1393 and it's goals).
This definitely aligns more with what I was thinking, but I wonder if it's a little less long-lasting.
This feature request was something that spawned out of a conversation around a future use-case. So in short, it's not something that needs to be done today as it won't immediately solve anything. After a discussion about general usefulness of enum strings, this issue was created. I found #1393 when I was looking for this request already, and I was concerned around the talk of beta and how it may not be easily to support this kind of feature quickly. So I logged this in the hopes to pair-down to the request I was after. |
how about just having the default existing data types and then add a spec field called - name: colors
description: >-
The context directory within the source repository
choices:
- blue
- green and if the parameter type is array then we can do do mutli from the chocies @vdemeester ^^ |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
Does it make sense to keep this issue open, or should we focus on our attention specifically on #1393 and tackling param types as whole? |
Agreed @bobcatfish, #1393 will be able to cover this. This was not adopted at the rate that I had hoped it would be so we can lean into #1393 as the solution that will cover this scenario as well. |
Some of the template sections are not feature-request oriented, so I'll just give a trimmed down version. Today this does not exist as far as I can tell from the docs. There is a suggestion in #1393 (comment) but it's asking for a lot of other flexibility as well. Which might be a larger and slower to get ask.
Expected Behavior
I believe this format works for Pipeline params, Task params, and maybe even worth going to TriggerTemplates on the Trigger package if viable.
Additional Info
Expectation would be for the logic to match 1:1 each
values
option. Not sure if it needs to be only strings but since we don't really have non-string types today not sure there is a concern here. Either way, I think it can just be a 1:1 equality check against the list of items and if the types grow, it should "just strong equality match". So if support of Booleans or Numbers come down the line, it won't be a problem.The text was updated successfully, but these errors were encountered: