refactor: refactor ScaledJob to be used with scale_handler #4707

yoongon · 2023-06-18T12:29:41Z

Provide a description of what has been changed
Currently, ScaledJob is left at scalers_cache.go. Move this one into the scale_handler.go for consistent with ScaledObject.

FYI.

keda/pkg/scaling/cache/scalers_cache.go

Line 144 in bce3375

    
           // TODO needs refactor - move ScaledJob related methods to scale_handler, the similar way ScaledObject methods are

Checklist

I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #4781

github-actions · 2023-06-18T12:29:51Z

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

Add an entry in our changelog in alphabetical order and link related issue
Update the documentation, if needed
Add unit & e2e tests for your changes
GitHub checks are passing
Is the DCO check failing? Here is how you can fix DCO issues

Learn more about:

Our contribution guide

zroubalik

Thanks for tackling this, my ambition behind this is not just move these functions but also revise & clenup the code. Also some additional tests would be nice.

pkg/scaling/scale_handler.go

yoongon · 2023-06-26T09:10:29Z

@zroubalik
Could you check my comment again? Thanks.

zroubalik

@yoongon first of all, sorry for the delay, I have been busy with other stuff. Great job, could you please address the rest of the minor concerns? Thanks!

pkg/scaling/scale_handler.go

pkg/scaling/utils/metrics.go

zroubalik · 2023-07-07T10:57:55Z

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

zroubalik

@yoongon could you please also create an issue for this effort and link it in the Changelog? (Other section)

yoongon · 2023-07-08T07:11:50Z

@zroubalik
Thanks for the detail check. I've updated code based on your review and update changelog as well.
Please check and let me know if there is anything I can contribute more :)

BTW, one test hasn't passed, but I don't think it's related with this change.

zroubalik

Thanks, there are some leftovers, sorry for being so pedantic about this 😄

pkg/scaling/scaledjob/metrics.go

pkg/scaling/scale_handler.go

CHANGELOG.md

zroubalik · 2023-07-12T13:49:06Z

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

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

zroubalik · 2023-07-12T16:20:06Z

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

zroubalik

LGTM

Thanks for the effort! Really appreciate this :)

yoongon requested a review from a team as a code owner June 18, 2023 12:29

yoongon force-pushed the feature/refactor-handler2 branch 6 times, most recently from dfeea1b to e1eba2c Compare June 18, 2023 13:58

zroubalik reviewed Jun 18, 2023

View reviewed changes

pkg/scaling/scale_handler.go Outdated Show resolved Hide resolved

pkg/scaling/scale_handler.go Outdated Show resolved Hide resolved

pkg/scaling/scale_handler.go Outdated Show resolved Hide resolved

yoongon requested a review from zroubalik June 19, 2023 22:57

yoongon force-pushed the feature/refactor-handler2 branch 4 times, most recently from 2b36c23 to b057f22 Compare June 20, 2023 23:06

zroubalik reviewed Jul 7, 2023

View reviewed changes

yoongon force-pushed the feature/refactor-handler2 branch 2 times, most recently from f7c5ce1 to 25ff08f Compare July 8, 2023 05:53

yoongon mentioned this pull request Jul 8, 2023

Move ScaledJob logic under scale_handler.go #4781

Closed

yoongon force-pushed the feature/refactor-handler2 branch 2 times, most recently from 428b5a6 to ef3f726 Compare July 8, 2023 07:05

yoongon requested a review from zroubalik July 8, 2023 12:25

zroubalik reviewed Jul 10, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

yoongon force-pushed the feature/refactor-handler2 branch 3 times, most recently from 7720f5d to da6cd84 Compare July 12, 2023 13:42

yoongon force-pushed the feature/refactor-handler2 branch from da6cd84 to 99f31e4 Compare July 12, 2023 14:18

yoongon added 21 commits July 12, 2023 23:37

Refator to use GetMetricsAndActivityForScaler

9a6becb

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Implement test cases inside scale_handler_test.go

37e75b6

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

All tests work

9fce158

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Make all test cases runnable under scale_hanlder_test

b9818da

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Delete unnecessary lines

2d3ed5d

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Delete scalers_cache_tests.go after migration

145a357

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Remove methods at scalers_cache.go as well

b908313

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Change receiver name

764b4d8

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Sort module to pass statick analysis

7480fcd

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Use separate package for the method getTargetAverageValue

62ec041

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Relocate createMetricSpec and related test cases

c44fcfb

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Rearrange import module to pass static analysis

ce7c464

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Add tests for scaledJob identifier

d486a66

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

rename the package name and delete comments

0b862a1

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Move scaledJob metric logic under scaledJob package

ff5cd11

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Add change log

080651c

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Add CalculateQueueLengthAndMaxValue at scaledJob

8197f20

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Use renamed function getTargetAverageValue

40922d3

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Move createMetricSpec inside test code

68e6779

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Use different metric name to pass static analysis

f9344e9

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

Delete ineffectual assignment

4259185

Signed-off-by: Yoon Park <yoongon.park@gmail.com>

yoongon force-pushed the feature/refactor-handler2 branch from 16aec79 to 4259185 Compare July 12, 2023 14:37

zroubalik approved these changes Jul 12, 2023

View reviewed changes

zroubalik enabled auto-merge (squash) July 12, 2023 16:21

zroubalik merged commit 2a3d9f1 into kedacore:main Jul 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: refactor ScaledJob to be used with scale_handler #4707

refactor: refactor ScaledJob to be used with scale_handler #4707

yoongon commented Jun 18, 2023 •

edited by zroubalik

Loading

github-actions bot commented Jun 18, 2023

zroubalik left a comment

yoongon commented Jun 26, 2023

zroubalik left a comment

zroubalik commented Jul 7, 2023 •

edited by github-actions bot

Loading

zroubalik left a comment

yoongon commented Jul 8, 2023 •

edited

Loading

zroubalik left a comment

zroubalik commented Jul 12, 2023 •

edited by github-actions bot

Loading

zroubalik commented Jul 12, 2023 •

edited by github-actions bot

Loading

zroubalik left a comment •

edited

Loading

refactor: refactor ScaledJob to be used with scale_handler #4707

refactor: refactor ScaledJob to be used with scale_handler #4707

Conversation

yoongon commented Jun 18, 2023 • edited by zroubalik Loading

Checklist

github-actions bot commented Jun 18, 2023

zroubalik left a comment

Choose a reason for hiding this comment

yoongon commented Jun 26, 2023

zroubalik left a comment

Choose a reason for hiding this comment

zroubalik commented Jul 7, 2023 • edited by github-actions bot Loading

zroubalik left a comment

Choose a reason for hiding this comment

yoongon commented Jul 8, 2023 • edited Loading

zroubalik left a comment

Choose a reason for hiding this comment

zroubalik commented Jul 12, 2023 • edited by github-actions bot Loading

zroubalik commented Jul 12, 2023 • edited by github-actions bot Loading

zroubalik left a comment • edited Loading

Choose a reason for hiding this comment

yoongon commented Jun 18, 2023 •

edited by zroubalik

Loading

zroubalik commented Jul 7, 2023 •

edited by github-actions bot

Loading

yoongon commented Jul 8, 2023 •

edited

Loading

zroubalik commented Jul 12, 2023 •

edited by github-actions bot

Loading

zroubalik commented Jul 12, 2023 •

edited by github-actions bot

Loading

zroubalik left a comment •

edited

Loading