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

Upgrade TriggerMetadata to an interface #4771

Open
Eldarrin opened this issue Jul 4, 2023 · 12 comments
Open

Upgrade TriggerMetadata to an interface #4771

Eldarrin opened this issue Jul 4, 2023 · 12 comments
Labels
feature-request All issues for new features that have not been committed to needs-discussion stale-bot-ignore All issues that should not be automatically closed by our stale bot

Comments

@Eldarrin
Copy link
Contributor

Eldarrin commented Jul 4, 2023

Proposal

I've noticed when writing scalers I need to use TriggerMetadata (map[string]string) which just give KVPs. However I end up using arrays so just end up with comma separated strings. This makes charts etc for deploying scalers harder.

Could we add a new TriggerMetadata to an interface or similar so that arrays can be actual arrays in the manifests when deploying scaler instances?

example:

TriggerMetadata:

demands: "test,test1,test2"

vs:

demands:
  - test
  - test1
  - test2

Use-Case

Easier management of actual scalers through helm etc, especially when using a lot of them.

Is this a feature you are interested in implementing yourself?

Maybe

Anything else?

I can do it, but I suspect some design thought is needed here first.

@Eldarrin Eldarrin added feature-request All issues for new features that have not been committed to needs-discussion labels Jul 4, 2023
@Eldarrin
Copy link
Contributor Author

Eldarrin commented Jul 4, 2023

added a sample of my thinking

@zroubalik
Copy link
Member

Yeah, the initial decision behind TriggerMetadata structure is not ideal. It would be much better if we use a different approach. For example the one you proposed.

My only concern is backwards compatibility.

@Eldarrin
Copy link
Contributor Author

Eldarrin commented Jul 7, 2023

Well the only way to maintain backwards is to add a new variable to the scalerconfig or we need to rewrite every scaler to match. Don't see another way.

@Eldarrin
Copy link
Contributor Author

Eldarrin commented Jul 8, 2023

@zroubalik I can design something that will work with 2.x if you are interested, but happy to leave it to 3.x

@tomkerkhove
Copy link
Member

Stupid question - Why can't we introduce a new API version that uses this new design? Implementation wide input scheme of TriggerAuth should be decouple from how scalers use that info, no?

@stale
Copy link

stale bot commented Sep 8, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Sep 8, 2023
@JorTurFer
Copy link
Member

@zroubalik ?

@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Sep 9, 2023
Copy link

stale bot commented Nov 8, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Nov 8, 2023
Copy link

stale bot commented Nov 16, 2023

This issue has been automatically closed due to inactivity.

@stale stale bot closed this as completed Nov 16, 2023
@github-project-automation github-project-automation bot moved this from Proposed to Ready To Ship in Roadmap - KEDA Core Nov 16, 2023
@zroubalik zroubalik reopened this Nov 21, 2023
@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Nov 21, 2023
@github-project-automation github-project-automation bot moved this from Ready To Ship to Proposed in Roadmap - KEDA Core Nov 21, 2023
Copy link

stale bot commented Jan 20, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Jan 20, 2024
Copy link

stale bot commented Jan 27, 2024

This issue has been automatically closed due to inactivity.

@stale stale bot closed this as completed Jan 27, 2024
@github-project-automation github-project-automation bot moved this from Proposed to Ready To Ship in Roadmap - KEDA Core Jan 27, 2024
@zroubalik zroubalik reopened this Jan 29, 2024
@github-project-automation github-project-automation bot moved this from Ready To Ship to Proposed in Roadmap - KEDA Core Jan 29, 2024
@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Jan 29, 2024
Copy link

stale bot commented Mar 30, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Mar 30, 2024
@zroubalik zroubalik added stale-bot-ignore All issues that should not be automatically closed by our stale bot and removed stale All issues that are marked as stale due to inactivity labels Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request All issues for new features that have not been committed to needs-discussion stale-bot-ignore All issues that should not be automatically closed by our stale bot
Projects
Status: Proposed
Development

No branches or pull requests

4 participants