Updating getSlotCounts to use sessions allowed - #3183
Merged
Conversation
added 2 commits
November 29, 2016 09:37
Switching getSlotCounts to use the concurrent sessions when counting the total slots. Old method would count all browser options so the free/total counts could be much higher than was actually available.
lukeis
reviewed
Nov 29, 2016
|
|
||
| totalSlots += 1; | ||
| } | ||
| totalSlots += proxy.getTestSlots().size() > proxy.getMaxNumberOfConcurrentTestSessions() ? |
Member
There was a problem hiding this comment.
totalSlots += Math.min(proxy.getMaxNumberOfConcurrentTestSessions(), proxy.getTestSlots().size());
Member
There was a problem hiding this comment.
reducing this further... could just get rid of the two other variables and just have:
freeSlots += Math.min(proxy.getMaxNumberOfConcurrentTestSessions(), proxy.getTestSlots().size()) - proxy.getTotalUsed();
and yeah, i agree still need the += :)
Contributor
Author
There was a problem hiding this comment.
I think having the totalSlots is still valuable info. My goal was getting some better utilization numbers for our grid.
How about removing the freeSlots variable and just doing
result.addProperty("free", totalSlots - usedSlots);
Member
|
LGTM, once comments addressed. :) |
mitchloudenbeck
commented
Nov 29, 2016
|
|
||
| totalSlots += 1; | ||
| } | ||
| totalSlots += proxy.getTestSlots().size() > proxy.getMaxNumberOfConcurrentTestSessions() ? |
Contributor
Author
There was a problem hiding this comment.
Don't we still want += here? It is summing all the slots/sessions of all the nodes connected to the hub.
Member
There was a problem hiding this comment.
yes, sorry.. I mis-read the code and missed the for loop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Xin the preceding checkbox, I verify that I have signed the Contributor License AgreementSwitching getSlotCounts to use the maxSessions when counting the total
slots. Old method would count all browser options so the free/total counts
could be much higher than what was actually available.