-
Notifications
You must be signed in to change notification settings - Fork 113
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
Config mgmt Get Node Metadata Counts #3360
Conversation
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
6a31ba6
to
c749489
Compare
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
9002121
to
f98ade1
Compare
@@ -26,5 +26,5 @@ type BackendError struct { | |||
|
|||
// NewBackendError - create a new backend error | |||
func NewBackendError(format string, args ...interface{}) *BackendError { | |||
return &BackendError{New(Backend, fmt.Sprint(format, args))} | |||
return &BackendError{New(Backend, fmt.Sprintf(format, args...))} |
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.
The arguments were not being used before.
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
…mt_field_term_counts Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
…mt_field_term_counts
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
api/external/cfgmgmt/cfgmgmt.proto
Outdated
/* | ||
GetNodesFieldValueCounts | ||
|
||
Returns a list fields with value counts |
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.
nit: "list of fields" ?
maybe include an example (of response and request) for clarity?
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.
maybe also add a sentence about how the counts are affected by the filters? (e.g. counts respect all filters except their own or some other wording that makes sense?)
[19][default:/src:130]# curl -s -f --insecure -H "api-token: $(get_api_token)" "https://localhost/api/v0/cfgmgmt/nodes_field_value_counts?terms=platform&terms=status" | jq
{
"fields": [
{
"terms": [
{
"term": "mac_os_x 10.11.5",
"count": 28
},
{
"term": "linux 8.9",
"count": 1
},
{
"term": "macos 8.9",
"count": 1
},
{
"term": "windows 8.9",
"count": 1
}
],
"field": "platform"
},
{
"terms": [
{
"term": "missing",
"count": 29
},
{
"term": "failure",
"count": 2
}
],
"field": "status"
}
]
}
[20][default:/src:0]# curl -s -f --insecure -H "api-token: $(get_api_token)" "https://localhost/api/v0/cfgmgmt/nodes_field_value_counts?terms=platform&terms=status&filter=platform:linux*" | jq
{
"fields": [
{
"terms": [
{
"term": "mac_os_x 10.11.5",
"count": 28
},
{
"term": "linux 8.9",
"count": 1
},
{
"term": "macos 8.9",
"count": 1
},
{
"term": "windows 8.9",
"count": 1
}
],
"field": "platform"
},
{
"terms": [
{
"term": "missing",
"count": 1
}
],
"field": "status"
}
]
}
working well locally! i tried a few other api calls too:
|
api/external/cfgmgmt/cfgmgmt.proto
Outdated
``` | ||
*/ | ||
rpc GetNodesFieldValueCounts(cfgmgmt.request.NodesFieldValueCounts) returns (cfgmgmt.response.NodesFieldValueCounts) { | ||
option (google.api.http).get = "/cfgmgmt/nodes_field_value_counts"; |
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.
@chef/ux-team do you have any strong opinions about this api?
i can't think of anything clearer than nodes_field_value_counts
but i also kinda stared at it sideways for a min after reading the endpoint it
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.
these are some random thoughts i had about this...
(but i dont have strong opinions)
- i could not look at the endpoint name and understand what it was for
- i think
term
might better work asvalue
- and
field
astype
- then maybe
node_metadata_counts
as the name
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.
ooooh, i like node_metadata_counts
!
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.
Yeah, I have had a lot of problems naming this function. Also even talking about what it does. I will update it to the above suggestions.
I renamed all the function names and parameters. I also added more comments and descriptions to the functions. |
ff50714
to
6d38c9a
Compare
6d38c9a
to
32f1a64
Compare
🔩 Description: What code changed, and why?
Adding a new endpoint that for a node's field type provides distinct values and counts.
⛓️ Related Resources
#3344
👟 How to Build and Test the Change
build components/config-mgmt-service && build components/automate-gateway && start_all_services
1.
generate_chef_run_example | jq --arg s "failure" --arg p "windows" '.status = $s | .node.automatic.platform=$p' | send_chef_data_raw
1.
generate_chef_run_example | jq --arg s "success" --arg p "linux" '.status = $s | .node.automatic.platform=$p' | send_chef_data_raw
1.
generate_chef_run_example | jq --arg s "failure" --arg p "macos" '.status = $s | .node.automatic.platform=$p' | send_chef_data_raw
curl -s -f --insecure -H "api-token: $(get_api_token)" "https://localhost/api/v0/cfgmgmt/nodes_field_value_counts?terms=platform&terms=status" | jq
✅ Checklist