View one beacon per host prioritization by transport #1640
Conversation
Summary
Previous Results
Insights
Slowest Tests
🎉 No failed tests in this run. | 🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
| </FormLabel> | ||
| <Switch id='isOnePerHost' className="pt-1" colorScheme="purple" onChange={() => setViewOnlyOnePerHost((value) => !value)} /> | ||
| </div> | ||
| <Tooltip label="Shows only the best beacon per host, prioritizing admin privileges and better transports (GRPC > HTTP > DNS)" placement="top"> |
There was a problem hiding this comment.
Nit:
Change to something like:
'Show only one beacon per host, prioritizing admin privileges and more reliable transports.'
Its ok to not list out the specific order.
There was a problem hiding this comment.
Additionally can you add these props to the tooltip
I think it may be good to potentially put the tooltip on the bottom, unless there is a reason you put it on top?
bg="white"
color="gray.600"
borderWidth="1px"
borderColor="gray.100"
There was a problem hiding this comment.
No reason for top, just used to most tooltips being on top rather than bottom.
| // Transport priority: GRPC > HTTP1 > DNS | ||
| const getTransportPriority = (transport: string | undefined): number => { | ||
| if (!transport) return 0; | ||
| const lowerTransport = transport.toLowerCase(); |
There was a problem hiding this comment.
Transports are ENUMS . We shouldn't swap to lowercase and match on lowercase. Instead it seems like we could just check for the exact expected value.
I'd save toLowerCase() for scenarios where the value in the field is user input and we aren't exact matching.
|
LGTM, Thanks for the addition! |
cmp5987
deleted the
1627-feature-view-one-beacon-per-host-prioritization-by-transport
branch
What type of PR is this?
/kind feature
What this PR does / why we need it:
Changes the "View one beacon per host" feature on the create quest page to prioritize by both privilege level and transport type. It will always prefer the highest privilege level, and will choose GRPC over HTTP over DNS given both beacons are the same privilege level.
Example:
Which issue(s) this PR fixes:
Fixes #1627