-
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
fix: Added port information check in ClusterIPAddress #4162
fix: Added port information check in ClusterIPAddress #4162
Conversation
Signed-off-by: ithesadson <thesadson@gmail.com>
Signed-off-by: ithesadson <thesadson@gmail.com>
/run-e2e cassandra* |
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, thanks for the contribution!
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.
One more thing, do you think you can add unit test for this fix? https://github.com/kedacore/keda/blob/main/pkg/scalers/cassandra_scaler_test.go
Signed-off-by: ithesadson <thesadson@gmail.com>
I added the unit test. But e2e-tests has been running for about 5 days and has not finished yet. Is there any problem with my unit test? |
/run-e2e cassandra* |
@ithesadson maintainers need to start e2e tests, I've just triggered them |
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
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, thanks for the fix!
Signed-off-by: ithesadson thesadson@gmail.com
Since the change I made before was a breaking change, I just fix the mistake and send a pr again.
Issue : #4110
Previous PR: #4079
If the clusterIPAddress contains port information, checking if the port information is entered in the clusterIPAddress will cause some errors.
Error example in current code: User enters the following values.
clusterIPAddress: "https://cassandra.test/"
port: "9042"
->Expected clusterIPAddress : https://cassandra.test:9042/
The actual clusterIPAddress: https://cassandra.test/
Checklist