Skip to content
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.

Sign up for GitHub

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

[WIP] Add test node #377

Closed
wants to merge 3 commits into from
Closed

[WIP] Add test node #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1de9c4f
Add tests for nodes
Bzil Jul 4, 2024
03249ce
Add tests for nodes
Bzil Jul 4, 2024
a37033c
Add tests for ex on index page
Bzil Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions spec/namespaces/nodes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,21 @@ paths:
responses:
'200':
$ref: '#/components/responses/nodes.info@200'
/_nodes/{metric}/stats:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Read https://opensearch.org/docs/latest/api-reference/nodes-apis/index/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path already exists in the spec, it is /_nodes/{node_id}/stats and is part of the nodes.stats operation not nodes.info

get:
operationId: nodes.info.1
x-operation-group: nodes.info
x-version-added: '1.0'
description: Returns information about nodes in the cluster.
externalDocs:
url: https://opensearch.org/docs/latest/api-reference/nodes-apis/nodes-info/
parameters:
- $ref: '#/components/parameters/nodes.info::path.metric'
- $ref: '#/components/parameters/nodes.info::query.flat_settings'
- $ref: '#/components/parameters/nodes.info::query.timeout'
responses:
'200':
$ref: '#/components/responses/nodes.info@200'
/_nodes/{node_id}:
get:
operationId: nodes.info.2
Expand Down
11 changes: 7 additions & 4 deletions spec/schemas/nodes.info.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,24 @@ components:
- aggregations
- indices
- search_pipelines
- _all
- _cluster_manager
Comment on lines +24 to +25
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are node filters not metric names

ResponseBase:
allOf:
- type: object
properties:
cluster_name:
$ref: '_common.yaml#/components/schemas/Name'
nodes:
type: object
additionalProperties:
$ref: '#/components/schemas/NodeInfo'
oneOf:
- type: object
$ref: '#/components/schemas/NodeInfo'
- type: object
additionalProperties: {}
_nodes:
$ref: 'nodes._common.yaml#/components/schemas/NodesResponseBase'
required:
- cluster_name
- nodes
- _nodes
Comment on lines 32 to 42
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are likely due to the duplicate path and having the wrong schemas, please revert them and test again with other changes made.

NodeInfo:
type: object
Expand Down
50 changes: 50 additions & 0 deletions tests/_core/nodes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,53 @@ chapters:
method: GET
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be moved under tests/nodes/ and split into info.yaml and stats.yaml

response:
status: 200
- synopsis: Nodes thread_pool metric with a json response.
path: /_nodes/{metric}
method: GET
parameters:
metric:
- thread_pool
response:
status: 200
- synopsis: Nodes jvm and thread_pool metric with a json response.
path: /_nodes/{metric}
method: GET
parameters:
metric:
- jvm
- thread_pool
response:
status: 200
- synopsis: Nodes _all metric with a json response.
path: /_nodes/{metric}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is technically /_nodes/{node_id}

method: GET
parameters:
metric:
- _all
response:
status: 200
- synopsis: Nodes _cluster_manager metric with a json response.
path: /_nodes/{metric}/stats
method: GET
parameters:
metric:
- _cluster_manager
response:
status: 200
- synopsis: Nodes _cluster_manager metric with a json response.
path: /_nodes/{metric}/stats
method: GET
parameters:
metric:
- data:true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dblock do you have any idea how to implement a fix for this test ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error?

FAILED  Nodes _cluster_manager metric with a json response. 
            FAILED  PARAMETERS 
                FAILED  metric (data/0 must be equal to one of the allowed values)

The data:true value doesn't seem like a valid metric.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's seems weird yes, but it's define in documentation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it read like a JSON value? Quote it, "data:true"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I do that, that implies that I have to put it in the enum and therefore do I have to do the same for data:false and the other filters?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that yes? There's really no such thing as booleans in URL paths AFAIK, this is a very creative use of the API.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be using /_nodes/{node_id}/stats

response:
status: 200
- synopsis: Nodes _cluster_manager metric with a json response.
path: /_nodes/{metric}/stats
method: GET
parameters:
metric:
- _all
- cluster_manager:false
response:
status: 200
Loading