add top to job requests api

[shayki5]

Provide a description of what has been changed

Checklist

• When introducing a new scaler, I agree with the scaling governance policy
• I have verified that my change is according to the deprecations & breaking changes policy
• Tests have been added
• Changelog has been updated and is aligned with our changelog requirements
• A PR is opened to update the documentation on (repo) (if applicable)
• Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #4324

[JorTurFer]

Hi @shayki5
I ❤️ this improvement!
Should we allow users to set their own values for the top (keeping 250 or whatever as default)? I mean, end-users can increase the timeout using env vars and maybe there are some user cases in really large orgs where 250 isn't enough, maybe giving the option to set the top value (in addition to the already existing option for increasing the timeout) would fulfil that requirement.
WDYT?

[shayki5]

Hi @JorTurFer, thank you for your quick response!
Yea, it's a good idea, it could be a variable, I just need to check how to do it...
I thought 250 it's enough because need more than 250 waiting jobs in one pool for the scale not to work properly, but as you said, maybe in large orgs it's should be the case.

[zroubalik]

I agree with @JorTurFer, we should add a property for this :) Thanks for the contribution!

[shayki5]

@zroubalik @JorTurFer I'm not familiar with go but I tried to add it as a parameter, can you review please? thank you!

[JorTurFer]

Looking good! Could you add this new field to the parsing tests?
As we are adding a new parameter, a PR to docs is also required explaining the new parameter. This is the file you need to edit https://github.com/kedacore/keda-docs/blob/main/content/docs/2.10/scalers/azure-pipelines.md

[shayki5]

@JorTurFer Sure, thank you so much for the parsing test, I added it.
I opened a PR for the docs: kedacore/keda-docs#1082

[JorTurFer]

Sure, thank you so much for the parsing test, I added it.

I didn't add the parsing test, I just suggested a change in the code to follow the sytle xD You still need to add a case here https://github.com/kedacore/keda/blob/main/pkg/scalers/azure_pipelines_scaler_test.go#L26

[shayki5]

Sure, thank you so much for the parsing test, I added it.

I didn't add the parsing test, I just suggested a change in the code to follow the sytle xD You still need to add a case here https://github.com/kedacore/keda/blob/main/pkg/scalers/azure_pipelines_scaler_test.go#L26

Oh sorry, my bad, this is what happens that today is the first day for you to write go ;) I just followed the demands syntax...
Which test should I add? I see the testAzurePipelinesMetadata checks if there are empty values but in this case the jobsToFetch it's optional, it can be empty.

[JorTurFer]

Which test should I add?

I think that we should cover 3 cases, empty (already covered xD), correctly provided and incorrectly provided. I mean, in case of empty we don't have anything to test, but we should test that if a correct value is set we don't raise any error and in case of wrong value set, we raise an error.
This can be done just adding the new 2 cases to the array

[shayki5]

Which test should I add?

I think that we should cover 3 cases, empty (already covered xD), correctly provided and incorrectly provided. I mean, in case of empty we don't have anything to test, but we should test that if a correct value is set we don't raise any error and in case of wrong value set, we raise an error. This can be done just adding the new 2 cases to the array

I understand, I tried to look in the existing tests but couldn't figure out how can I implement those tests :(

[JorTurFer]

You can check how this case (activationTargetPipelinesQueueLength malformed) is added: https://github.com/kedacore/keda/blob/main/pkg/scalers/azure_pipelines_scaler_test.go#L41-L42

Basically, you need to provide the scaler values (you can take this line as base) including the new parameter, in one case wrongly (for example, setting "a" instead of a number) and other case correctly. If you check the line options, there is a boolean to specify if the case should raise an error or not. The wrong case should have true in that field because it should raise an error, the correct case should have false because not error should be raised

[shayki5]

You can check how this case (activationTargetPipelinesQueueLength malformed) is added: https://github.com/kedacore/keda/blob/main/pkg/scalers/azure_pipelines_scaler_test.go#L41-L42

Basically, you need to provide the scaler values (you can take this line as base) including the new parameter, in one case wrongly (for example, setting "a" instead of a number) and other case correctly. If you check the line options, there is a boolean to specify if the case should raise an error or not. The wrong case should have true in that field because it should raise an error, the correct case should have false because not error should be raised

Thank you so much for your explanation! I added something, can you check..?

[JorTurFer]

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

[JorTurFer]

LGTM! Thanks for the improvement.
Please update the changelog (The comment I wrote) and also the test (the malformed case should raise an error, that's why the boolean has to be true)

[shayki5]

Fixed :) thank you so much!!

[JorTurFer]

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

[JorTurFer]

Thanks for the contribution!
This feature will be released as part of v2.10

[shayki5]

Your prompt response and coordination are greatly appreciated!
Do you know when v2.10 will be released?

[JorTurFer]

Do you know when v2.10 will be released?

Most probably this week