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 fetch top queries by id API #195

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

deshsidd
Copy link
Collaborator

@deshsidd deshsidd commented Jan 22, 2025

Description

This PR contains all the backend changes required to add the fetch top queries by id API.

This change is a prerequisite to the change on the UI to make sure we include the to, from and id in the url for the query/group details page and make an API call to fetch the query/group details rather than propagating the information from the Top N queries page.

Change comprises of the following:

  1. Update the local index reader to fetch queries from local index by id
  2. Update the TopQueriesService to filter based on id if id is provided
  3. Refactoring to make parts of the code more readable
  4. Update the unit tests based on the changes and add additional unit tests
  5. Model changes to add id to the search query record
  6. API layer changes to add id as an optional param to the top queries API

Testing:
Tested on localhost.

Get query/group by id:
image
Get query/group by id:
image
Get queries/groups without id with time range:
image
Get queries/groups without id and without time range:
image

Issues Resolved

closes: #159

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.

Signed-off-by: Siddhant Deshmukh <[email protected]>
@deshsidd deshsidd marked this pull request as ready for review January 22, 2025 22:33
Copy link
Member

@ansjcy ansjcy left a comment

Choose a reason for hiding this comment

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

The overall logic LGTM. merging it.

@ansjcy ansjcy merged commit 210ce5d into opensearch-project:main Jan 23, 2025
12 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 23, 2025
Signed-off-by: Siddhant Deshmukh <[email protected]>
(cherry picked from commit 210ce5d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
deshsidd pushed a commit that referenced this pull request Jan 23, 2025
(cherry picked from commit 210ce5d)

Signed-off-by: Siddhant Deshmukh <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.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.

[FEATURE] Add an ID Field to Search Query Records
2 participants