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.

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

add workload management querygroup api specs #649

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

ruai0511
Copy link
Contributor

Description

API spec for workload management, more specifically, for the query group lifecycle APIs.
Related PR: opensearch-project/OpenSearch#14680, opensearch-project/OpenSearch#14775, opensearch-project/OpenSearch#14735, opensearch-project/OpenSearch#14709

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>
Copy link
Contributor

github-actions bot commented Oct 29, 2024

Changes Analysis

Commit SHA: dae9c98
Comparing To SHA: 931a61a

API Changes

Summary

├─┬Paths
│ ├──[➕] path (9221:3)
│ └──[➕] path (9190:3)
└─┬Components
  ├──[➕] requestBodies (26973:7)
  ├──[➕] requestBodies (26967:7)
  ├──[➕] responses (30073:7)
  ├──[➕] responses (30099:7)
  ├──[➕] responses (30079:7)
  ├──[➕] responses (30088:7)
  ├──[➕] parameters (25354:7)
  ├──[➕] parameters (25340:7)
  ├──[➕] parameters (25347:7)
  ├──[➕] schemas (56280:7)
  ├──[➕] schemas (56361:7)
  ├──[➕] schemas (56372:7)
  └──[➕] schemas (56317:7)

Document Element Total Changes Breaking Changes
paths 2 0
components 13 0
  • Total Changes: 15
  • Additions: 15

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/11601758306/artifacts/2125468248

API Coverage

Before After Δ
Covered (%) 588 (57.59 %) 588 (57.59 %) 0 (0 %)
Uncovered (%) 433 (42.41 %) 433 (42.41 %) 0 (0 %)
Unknown 30 35 5

Copy link
Contributor

Spec Test Coverage Analysis

Total Tested
510 320 (62.75 %)

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>
@ruai0511
Copy link
Contributor Author

ruai0511 commented Oct 29, 2024

Workload Management plugin is only available in 2.17.0, but when I use the default ${OPENSEARCH_VERSION:-latest} in docker-compose file, it only gives 2.14.0. So I hardcoded 2.17.0 in it. I'm wondering if there's any better ways of doing it?

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Thanks!

The docker compose in tests/plugins/wlm is identical to the default one. We do custom plugins/xyz for when additional setup is required, so move this into tests/default and you don't need a special entry in the CI matrix.

spec/namespaces/query_group.yaml Outdated Show resolved Hide resolved
tests/plugins/wlm/query_group/query_group.yaml Outdated Show resolved Hide resolved
@ruai0511
Copy link
Contributor Author

ruai0511 commented Oct 30, 2024

Thanks!

The docker compose in tests/plugins/wlm is identical to the default one. We do custom plugins/xyz for when additional setup is required, so move this into tests/default and you don't need a special entry in the CI matrix.

The default docker compose uses ${OPENSEARCH_VERSION:-latest} which is 2.14.0, but we need 2.17.0 here. That's why I put the tests in plugin since we need Dockerfile (to install the workload-management plugin) and docker compose with version 2.17.0

@dblock
Copy link
Member

dblock commented Oct 30, 2024

Thanks!
The docker compose in tests/plugins/wlm is identical to the default one. We do custom plugins/xyz for when additional setup is required, so move this into tests/default and you don't need a special entry in the CI matrix.

The default docker compose uses ${OPENSEARCH_VERSION:-latest} which is 2.14.0, but we need 2.17.0 here. That's why I put the tests in plugin since we need Dockerfile (to install the workload-management plugin) and docker compose with version 2.17.0

You should upgrade that number, and it's 2.17 in CI: https://github.com/opensearch-project/opensearch-api-specification/blob/main/.github/workflows/test-spec.yml

@ruai0511
Copy link
Contributor Author

ruai0511 commented Oct 30, 2024

Thanks!
The docker compose in tests/plugins/wlm is identical to the default one. We do custom plugins/xyz for when additional setup is required, so move this into tests/default and you don't need a special entry in the CI matrix.

The default docker compose uses ${OPENSEARCH_VERSION:-latest} which is 2.14.0, but we need 2.17.0 here. That's why I put the tests in plugin since we need Dockerfile (to install the workload-management plugin) and docker compose with version 2.17.0

You should upgrade that number, and it's 2.17 in CI: https://github.com/opensearch-project/opensearch-api-specification/blob/main/.github/workflows/test-spec.yml

wlm is not installed by default. So we still need a separate dockerfile to install it and think it should be in the plugins directory

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>
@dblock
Copy link
Member

dblock commented Oct 30, 2024

Thanks!
The docker compose in tests/plugins/wlm is identical to the default one. We do custom plugins/xyz for when additional setup is required, so move this into tests/default and you don't need a special entry in the CI matrix.

The default docker compose uses ${OPENSEARCH_VERSION:-latest} which is 2.14.0, but we need 2.17.0 here. That's why I put the tests in plugin since we need Dockerfile (to install the workload-management plugin) and docker compose with version 2.17.0

You should upgrade that number, and it's 2.17 in CI: https://github.com/opensearch-project/opensearch-api-specification/blob/main/.github/workflows/test-spec.yml

wlm is not installed by default. So we still need a separate dockerfile to install it and it should be in the plugins directory

Sorry to be a pest, the plugin is called workload-management not wlm so let's make the folder tests/plugins/workload-management? The namespace is correct, since the route is wlm.

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Good work! Thanks for hanging in here with me.

@dblock dblock merged commit 5e3349a into opensearch-project:main Oct 30, 2024
22 checks passed
@@ -106,6 +106,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `config_id` and `config_id_list` to `/_plugins/_notifications/configs` query parameters ([#594](https://github.com/opensearch-project/opensearch-api-specification/pull/594))
- Added a release workflow triggered on a tag ([#635](https://github.com/opensearch-project/opensearch-api-specification/pull/635))
- Added API spec for query insights plugin ([#625](https://github.com/opensearch-project/opensearch-api-specification/pull/625))
- Added API specs for query groups lifecycle APIs ([#649](https://github.com/opensearch-project/opensearch-api-specification/pull/649))
Copy link
Member

Choose a reason for hiding this comment

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

@ruai0511 I missed this, but we released 0.1.0, could you please move this in another PR to the unreleased section? thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants