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

add support for StatefulSet #498

Closed
wants to merge 2 commits into from
Closed

Conversation

iyacontrol
Copy link
Contributor

No description provided.

@ahmelsayed
Copy link
Contributor

Sorry for the delay in reviewing this change. I think the feature in general makes sense. I'm a bit concerned about the amount of deployment similar code that's copied (for example, the leak fixed in this PR #684 will need similar code in the copied stateful set version). It might be difficult to unify the code in go, everytime I look at the PR I try to see how it could be unified, but I think a large refactor would be needed.

@zroubalik
Copy link
Member

Agree. Another point that we are changing the Spec of ScaledObject. If this feature is not urgent, we can wait till 2.0 which could implement this feature by default, see my proposal #703

@zroubalik
Copy link
Member

There is PR opened to introduce KEDA v2, this should support StatefulSet out of the box
#731

@tomkerkhove
Copy link
Member

tomkerkhove commented Apr 7, 2020

Should we close this one then @zroubalik?

@iyacontrol
Copy link
Contributor Author

There is PR opened to introduce KEDA v2, this should support StatefulSet out of the box
#731

woops!

@zroubalik
Copy link
Member

zroubalik commented Apr 8, 2020

Closing this one in favour of work done on #731, but thanks for the contribution @iyacontrol!

@zroubalik zroubalik closed this Apr 8, 2020
preflightsiren pushed a commit to preflightsiren/keda that referenced this pull request Nov 7, 2021
* Provide consistent docs for trigger authentication

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Align Redis scalers

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants