Implementation of sessionBrowserName and a fix for maxSessions (#2709 and #3061 #3062

Wolfe1 · 2022-05-23T20:53:01Z

Fixed the implementation of MaxSessions to now give an accurate count of nodes to scale up or down.
Implemented an optional parameter sessionBrowserName to allow users to specify a different name for their browser in the active sessions list. In this case this is the behavior of Edge where in the session queue it is called "MicrosoftEdge" and as an active session it is called "msedge".

Checklist

Commits are signed with Developer Certificate of Origin (DCO - learn more)
Tests have been added
A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
A PR is opened to update the documentation on (repo) (if applicable)
Changelog has been updated and is aligned with our changelog requirements

Fixes #2709
Fixes #3061

Docs PR: kedacore/keda-docs#764

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

JorTurFer

In general, looks good. Thanks for these improvements.
Only 2 things:

Unit tests are failing (Test_getCountFromSeleniumResponse)
Please, add an e2e test case covering this change (adding an edge node)

JorTurFer · 2022-05-24T06:19:19Z

/run-e2e selenium*
Update: You can check the progres here

zroubalik

Thanks! Next time please open a separate PR for each change, it is easier to review and maintain then.

JorTurFer · 2022-05-24T12:25:20Z

Thanks! Next time please open a separate PR for each change, it is easier to review and maintain then.

I think this has been due to me. Sorry, next time I'll remember it

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

JorTurFer · 2022-05-24T18:22:56Z

/run-e2e selenium*
Update: You can check the progres here

JorTurFer · 2022-05-24T19:02:11Z

/run-e2e selenium*
Update: You can check the progres here

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

� Conflicts: � CHANGELOG.md

JorTurFer · 2022-05-24T19:26:14Z

hey @Wolfe1
e2e tests are failing, I think because the current e2e image doesn't enqueue any job in edge nodes. Could you update it?
It's in the repository test-tools
TBH, I have no idea about how to update it :(
If I have to chose where, I'd say that something in this file, but I'm not 100% sure

Wolfe1 · 2022-05-24T19:35:50Z

@JorTurFer Really giving me the full tour of the keda repos haha

JorTurFer · 2022-05-24T19:38:20Z

@JorTurFer Really giving me the full tour of the keda repos haha

yes, and I only can say one thing, THANKS for the huge effort you are doing ❤️ ❤️ ❤️

Wolfe1 · 2022-05-24T19:43:31Z

@JorTurFer Haha no worry, enjoying it.

Still working through these unit tests, not quite sure why I am getting the results I am but at least I figured out how to run them locally now so you should get less spam from me.

JorTurFer · 2022-05-24T19:53:07Z

Still working through these unit tests, not quite sure why I am getting the results I am but at least I figured out how to run them locally now so you should get less spam from me.

Don't worry, I can show you how to do it locally xD. The easiest way is executing from root folder make test but that will run all unit tests. Most probably you prefer to run/debug only your tests to reduce the execution time and troubleshoot the issues, if you are using Visual Studio Code with the Golang extensions, you can just click on the contextual buttons:

JorTurFer · 2022-05-24T19:54:18Z

/run-e2e selenium*
Update: You can check the progres here

This reverts commit 2e2aa80. Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

tests/scalers/selenium-grid.test.ts

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Wolfe1 · 2022-05-25T15:12:24Z

Alright @JorTurFer I think we are ready for one last e2e run and it should be good to review

JorTurFer

LGTM!

JorTurFer · 2022-05-25T19:10:03Z

/run-e2e selenium*
Update: You can check the progres here

JorTurFer · 2022-05-25T19:10:45Z

Thanks a lot for these improvements! ❤️
Let's wait till e2e tests finish and I'll merge it then

Wolfe1 added 3 commits May 23, 2022 15:24

New maxSessions and sessionBrowserName logic

b2b47e1

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Changelog edits for kedacore#2709 and kedacore#3061

41f2871

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Add test for Edge

154efab

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Wolfe1 requested a review from a team as a code owner May 23, 2022 20:53

Formatting fixes

e6abeef

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

JorTurFer reviewed May 23, 2022

View reviewed changes

JorTurFer requested a review from zroubalik May 23, 2022 21:49

github-actions bot added the ok-to-merge label May 24, 2022

zroubalik reviewed May 24, 2022

View reviewed changes

e2e teat, metadata test for edge, tried to fix broken unit tests.

43350b4

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

github-actions bot removed the ok-to-merge label May 24, 2022

Wolfe1 added 2 commits May 24, 2022 14:19

Use of multiple sessions in the unit tests giving trouble

2e2aa80

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Merge branch 'main' of github.com:Wolfe1/keda into max_sessions_and_edge

35b5344

� Conflicts: � CHANGELOG.md

Wolfe1 mentioned this pull request May 24, 2022

Add edge to config kedacore/test-tools#75

Merged

2 tasks

github-actions bot added the ok-to-merge label May 24, 2022

Wolfe1 added 2 commits May 25, 2022 09:30

Revert "Use of multiple sessions in the unit tests giving trouble"

5f070c2

This reverts commit 2e2aa80. Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Unit test fixes

14e4664

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Wolfe1 force-pushed the max_sessions_and_edge branch from 7b77bd2 to 14e4664 Compare May 25, 2022 14:33

JorTurFer reviewed May 25, 2022

View reviewed changes

tests/scalers/selenium-grid.test.ts Outdated Show resolved Hide resolved

typo and additional tests for multi-browser situations

8f49f71

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Wolfe1 added 3 commits May 25, 2022 09:59

static code check fixes

fd69d47

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

reorder imports alphabetically

91e30f0

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

remove type declaration that can be inferred

3f6a4dc

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>

Wolfe1 requested review from zroubalik and JorTurFer May 25, 2022 15:31

JorTurFer approved these changes May 25, 2022

View reviewed changes

github-actions bot added ok-to-merge and removed ok-to-merge labels May 25, 2022

JorTurFer merged commit d5f857c into kedacore:main May 25, 2022

Wolfe1 deleted the max_sessions_and_edge branch May 25, 2022 22:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of sessionBrowserName and a fix for maxSessions (#2709 and #3061 #3062

Implementation of sessionBrowserName and a fix for maxSessions (#2709 and #3061 #3062

Wolfe1 commented May 23, 2022 •

edited

Loading

JorTurFer left a comment

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

zroubalik left a comment

JorTurFer commented May 24, 2022

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 24, 2022 •

edited

Loading

Wolfe1 commented May 24, 2022

JorTurFer commented May 24, 2022

Wolfe1 commented May 24, 2022

JorTurFer commented May 24, 2022

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

Wolfe1 commented May 25, 2022

JorTurFer left a comment

JorTurFer commented May 25, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 25, 2022

Implementation of sessionBrowserName and a fix for maxSessions (#2709 and #3061 #3062

Implementation of sessionBrowserName and a fix for maxSessions (#2709 and #3061 #3062

Conversation

Wolfe1 commented May 23, 2022 • edited Loading

Checklist

JorTurFer left a comment

Choose a reason for hiding this comment

JorTurFer commented May 24, 2022 • edited by github-actions bot Loading

zroubalik left a comment

Choose a reason for hiding this comment

JorTurFer commented May 24, 2022

JorTurFer commented May 24, 2022 • edited by github-actions bot Loading

JorTurFer commented May 24, 2022 • edited by github-actions bot Loading

JorTurFer commented May 24, 2022 • edited Loading

Wolfe1 commented May 24, 2022

JorTurFer commented May 24, 2022

Wolfe1 commented May 24, 2022

JorTurFer commented May 24, 2022

JorTurFer commented May 24, 2022 • edited by github-actions bot Loading

Wolfe1 commented May 25, 2022

JorTurFer left a comment

Choose a reason for hiding this comment

JorTurFer commented May 25, 2022 • edited by github-actions bot Loading

JorTurFer commented May 25, 2022

Wolfe1 commented May 23, 2022 •

edited

Loading

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 24, 2022 •

edited

Loading

JorTurFer commented May 24, 2022 •

edited by github-actions bot

Loading

JorTurFer commented May 25, 2022 •

edited by github-actions bot

Loading