-
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
Add azure-workload auth to MSSQL scaler #6161
base: main
Are you sure you want to change the base?
Conversation
877148b
to
e253199
Compare
633987e
to
8cd724e
Compare
dfbdac2
to
5a08a0d
Compare
Signed-off-by: Rick Brouwer <[email protected]>
783dd2d
to
a7d0e2c
Compare
Signed-off-by: rickbrouwer <[email protected]>
Hi @tomkerkhove I was refactoring the MSSQL scaler to match the new metadata guidelines. I thought I would also nice to add the requested Azure workload feature. I do not work with Azure myself and I mainly have to rely on reading about it. The unittest is successful, but I would like to see this very well checked. Could you check this PR? If I'm completely off track I could also leave it at just refactoring. |
Thanks a ton! Sadly, I don't do code contributions here so maybe @kedacore/keda-contributors can help with review |
Hi @pauldotyu! I saw that you did a lot of work for the Azure workload identity, for example with the RabbitMQ. I was refactoring the MSSQL scaler to match the new metadata guidelines. I thought I would also nice to add the #6104 Azure workload feature. I do not work with Azure myself and I mainly have to rely on reading about it. The unittest is successful, but I would like to see this very well checked. Is this something you can look into? If I'm completely off track, please let me know. |
Let's not poke random people and wait for the group that I have mentioned please =) |
Also, if the PR is ready for review - Please publish it. Draft PRs are typically WIP PRs |
my sincere apologies |
No worries |
/run-e2e * |
I think that we could cover this with e2e test as we have an Azure subscription. Could you open a PR to the testing infra repo to spin up and configure the database? |
Is a feature request in https://github.com/kedacore/testing-infrastructure ok? |
yes 😄 |
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.
@rickbrouwer @JorTurFer any update on this please?
The testing infrastructure has to be deployed but @rickbrouwer needs help with that and I've not had time yet |
Checklist
Fixes #6104
Relates to #5797