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.

Sign up for GitHub

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

Jump to bottom

refactor gcp stackdriver scaler #6462

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

refactor gcp stackdriver scaler #6462

wants to merge 10 commits into from

Conversation

omerap12
Copy link
Contributor

@omerap12 omerap12 commented Jan 2, 2025

Provide a description of what has been changed

Checklist

Fixes #

Relates to #5797

@omerap12
refactor gcp stackdriver scaler

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
6314664 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12 omerap12 requested a review from a team as a code owner January 2, 2025 15:34
@zroubalik
Copy link
Member

zroubalik commented Jan 2, 2025 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@omerap12
Merge branch 'main' into refactor-gcp-stackdriver-scaler
5879a56
@omerap12
lint
8f1127f 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
wozniakjan
wozniakjan approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thank you!

minor nits below

@wozniakjan
Copy link
Member

wozniakjan commented Jan 6, 2025 •
edited
Loading

just noticed the imports are not sorted the way linter is configured, can you please run following command? (or address the imports sort manually :))

gci write . --skip-generated -s standard -s default -s prefix(github.com/kedacore/keda)

I will retrigger the e2e tests after that

omerap12 and others added 2 commits January 6, 2025 15:39
@omerap12 @wozniakjan
optional is redundant with default
9adc070 
Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com>
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
lint
27b69c7 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
Copy link
Contributor Author

omerap12 commented Jan 6, 2025

just noticed the imports are not sorted the way linter is configured, can you please run following command? (or address the imports sort manually :))

gci write . --skip-generated -s standard -s default -s prefix(github.com/kedacore/keda)

I will retrigger the e2e tests after that

done

@omerap12
Copy link
Contributor Author

Friendly ping :) @zroubalik @wozniakjan

@dttung2905
Copy link
Contributor

dttung2905 commented Jan 13, 2025 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@wozniakjan
Copy link
Member

wozniakjan commented Jan 14, 2025 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

@stale Stale
Copy link

stale bot commented Mar 15, 2025

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 15, 2025
@wozniakjan
Copy link
Member

wozniakjan commented Mar 17, 2025 •
edited
Loading

looks like this breaks the e2e tests

	Execution of tests/scalers/gcp/gcp_stackdriver_workload_identity/gcp_stackdriver_workload_identity_test.go, has failed after "three" attempts

you can check for more logs
https://github.com/kedacore/keda/actions/runs/12762977209/job/35572602940

@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Mar 17, 2025
zroubalik
zroubalik reviewed Mar 26, 2025
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@omerap12 any update here, please?

@omerap12
Copy link
Contributor Author

@omerap12 any update here, please?

Yeah, sorry about that. I'll take a look this weekend.

@omerap12
typo
6f504ca 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
Copy link
Contributor Author

/run-e2e gcp

@JorTurFer
Copy link
Member

JorTurFer commented Mar 30, 2025 •
edited by github-actions bot
Loading

/run-e2e gcp
Update: You can check the progress here

omerap12 and others added 3 commits March 30, 2025 18:31
@omerap12 @JorTurFer
Update pkg/scalers/gcp_stackdriver_scaler.go
f9b317a 
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
revert
392b23c 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
projectID to projectId
aedb174 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
Copy link
Contributor Author

Ill squash when approved ( if needed )

@JorTurFer
Copy link
Member

JorTurFer commented Mar 30, 2025 •
edited by github-actions bot
Loading

/run-e2e stackdriver
Update: You can check the progress here

@omerap12
fixed unit tests
0da5179 
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@omerap12
Copy link
Contributor Author

Can you re run it now? @JorTurFer

@zroubalik
Copy link
Member

zroubalik commented Mar 31, 2025 •
edited by github-actions bot
Loading

/run-e2e stackdriver
Update: You can check the progress here

@omerap12
Copy link
Contributor Author

omerap12 commented Apr 3, 2025

@zroubalik I believe we can merge this now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik

@wozniakjan wozniakjan

@JorTurFer JorTurFer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@omerap12 @zroubalik @wozniakjan @dttung2905 @JorTurFer