-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ui] Fix: Wildcard-datacenter system/sysbatch jobs stopped showing client links/chart #16274
[ui] Fix: Wildcard-datacenter system/sysbatch jobs stopped showing client links/chart #16274
Conversation
Ember Asset Size actionAs of 9d2f67b Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
@@ -26,7 +26,10 @@ export default function jobClientStatus(nodesKey, jobKey) { | |||
|
|||
// Filter nodes by the datacenters defined in the job. | |||
const filteredNodes = nodes.filter((n) => { | |||
return job.datacenters.indexOf(n.datacenter) >= 0; | |||
return ( | |||
job.datacenters.includes('*') || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*
in datacenters is not like namespace, where *
is a special wildcard to match all. In datacenters it's actually a glob pattern so, for example, datacenters = ["dc*"]
would match dc1
and dc2
but not us-east-1
. This check would return all clients, even the ones not matched by the pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh thanks for keeping an eye out. I'll update to match on glob.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to @lgfa29 -- you probably want to extract the logic from _doesMatchPattern
into a utility and use it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that looks pretty good! If you need some test cases to match with the Go implementation:
https://github.com/ryanuber/go-glob/blob/master/glob_test.go
Ember Test Audit comparison
|
@@ -26,7 +26,10 @@ export default function jobClientStatus(nodesKey, jobKey) { | |||
|
|||
// Filter nodes by the datacenters defined in the job. | |||
const filteredNodes = nodes.filter((n) => { | |||
return job.datacenters.indexOf(n.datacenter) >= 0; | |||
return ( | |||
job.datacenters.includes('*') || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to @lgfa29 -- you probably want to extract the logic from _doesMatchPattern
into a utility and use it here.
51e2248
to
df68c96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. There's a large block of commented code that should be removed.
9e57b1f
to
9d2f67b
Compare
Will now return all clients when
datacenters = ["*"]
, relevant clients when["dc*"]
, etc. Otherwise will use the same filter as before.