-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Redis host port params #710
Conversation
Thanks! Let me know when the PR is up for the docs. Can you add some tests as well please? |
Pr for documentation created kedacore/keda-docs#125 |
Tests added! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one minor thing. Could you please add a test to check behaviour when address
, host and
port` are specified at the same time?
@zroubalik test added. |
@inuyasha82 thanks, looking good! One thing left, could you please sing your commit? See the DCO check failed. https://github.com/kedacore/keda/blob/master/CONTRIBUTING.md#developer-certificate-of-origin-signing-your-work |
@zroubalik i tried to solve signoff the changes, but when i run the signoff command i'm getting a conflict errorr with the following error:
Of course i tried to fix the conflict following the instrructions, but everytime i try to continue the rebase it still complain about adding the file (step that i have done):
I trried with and without committing the merge (that according to the instructions commit is not necessary. Plus if i do git status:
but then git rebase --continue still complains... |
sometimes it could be tricky, especially if you are performing merge during your work and not rebase. I'd recommend you to checkout your branch again, squash all your commits into one and sing this commit, squashing: https://www.internalpointers.com/post/squash-commits-into-one-git If you have further problems, you can ask me on #keda slack. |
* Add host/port parameter for redis scaler * Remove debug print * Use always address variable for getRedisListLength Co-authored-by: Ivan Gualandri <ivangual@ie.ibm.com> Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
Signed-off-by: Michael Droessler <michael.droessler@cunamutual.com> Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
* Provide project governance Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com> * Update sections Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com> Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
Signed-off-by: Ahmed ElSayed <ahmed@elsayed.io> Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com> Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com> Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
Signed-off-by: Ivan Gualandri <ivan.gualandri@gmail.com>
Ok probably better just to creaet a new branchand make a single commit and create a new PR, i see that this one has been completely messed up :) |
Signed-off-by: Ara Pulido <ara.pulido@datadoghq.com>
This should implement #694
How does it works:
now the redis scaler accept 2 new parameters: host and port in addition of the already existing.
If address is specified it use the address, otherwise it checks the host and port parameters, they must be difned both, otherwise an error will be returned.
If address and host and port are all specified, address will get priority.
I'll update the documentaton and create a PR soon.
Checklist
Fixes #694