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

Create health_stats API for query insights #122

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

ansjcy
Copy link
Member

@ansjcy ansjcy commented Sep 18, 2024

Description

Create healthstats API for query insights, using the data model from #120

Issues Resolved

Related to #9

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@ansjcy
Copy link
Member Author

ansjcy commented Sep 18, 2024

The build will fail until we rebase #120

@ansjcy ansjcy changed the title Create healthstats API for query insights Create health_stats API for query insights Sep 18, 2024
@deshsidd
Copy link
Collaborator

Looks like builds/tests and hygiene failing. Now that we have merged #120 - Let me know when this is ready to review @ansjcy

@ansjcy ansjcy force-pushed the health-api-for-qi branch 2 times, most recently from ec73491 to 1d8e313 Compare September 23, 2024 19:57
@ansjcy
Copy link
Member Author

ansjcy commented Sep 23, 2024

@deshsidd just rebased and this PR should be ready.

@@ -114,13 +114,13 @@ public void testGetExecutorBuilders() {

public void testGetRestHandlers() {
List<RestHandler> components = queryInsightsPlugin.getRestHandlers(Settings.EMPTY, null, null, null, null, null, null);
assertEquals(1, components.size());
assertEquals(2, components.size());
assertTrue(components.get(0) instanceof RestTopQueriesAction);
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the sake of completeness can verify that components.get(1) is as expected.

assertTrue(components.get(0) instanceof RestTopQueriesAction);
}

public void testGetActions() {
List<ActionPlugin.ActionHandler<? extends ActionRequest, ? extends ActionResponse>> components = queryInsightsPlugin.getActions();
assertEquals(1, components.size());
assertEquals(2, components.size());
assertTrue(components.get(0).getAction() instanceof TopQueriesAction);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Signed-off-by: Chenyang Ji <cyji@amazon.com>
@deshsidd
Copy link
Collaborator

LGTM

@ansjcy ansjcy merged commit e4c6b8f into opensearch-project:main Sep 25, 2024
15 of 16 checks passed
@ansjcy ansjcy deleted the health-api-for-qi branch September 25, 2024 18:25
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 27, 2024
Signed-off-by: Chenyang Ji <cyji@amazon.com>
(cherry picked from commit e4c6b8f)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ansjcy added a commit to ansjcy/query-insights that referenced this pull request Sep 27, 2024
Signed-off-by: Chenyang Ji <cyji@amazon.com>
ansjcy added a commit that referenced this pull request Sep 27, 2024
Signed-off-by: Chenyang Ji <cyji@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants