-
Notifications
You must be signed in to change notification settings - Fork 894
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 data summary panel in discover #8186
Conversation
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
pretty awesome! i have comments and questions but i like how it's looking. |
can you make sure to include a changelog? you might need to give the changelog bot permissions though. just will need to follow the directions given in the link |
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@joshuali925 @kavilla I've addressed your comments, could you help review again? |
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.
lgtm
const RESPONSE_TEXT = 'response'; | ||
httpMock.post.mockResolvedValue(RESPONSE_TEXT); | ||
renderQueryAssistSummary(COLLAPSED.NO); | ||
await sleep(2000); |
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.
curious does it use jest fake timers or is this actually sleeping 2 seconds?
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.
It's actual sleep, I'll change it to 100ms.
src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
setSummary(response); | ||
reportCountMetric(SUCCESS_METRIC, 1); | ||
} catch (error) { | ||
reportCountMetric(SUCCESS_METRIC, 0); |
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.
What about having another event FAILED_METRIC
? It doesn't seem METRIC_TYPE.COUNT
is intend to be used in this way.
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.
In this way we can get success rate easily only using one metric
src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx
Outdated
Show resolved
Hide resolved
src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx
Outdated
Show resolved
Hide resolved
src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx
Outdated
Show resolved
Hide resolved
src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx
Outdated
Show resolved
Hide resolved
src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx
Outdated
Show resolved
Hide resolved
<QueryAssistContext.Provider | ||
value={{ | ||
question, | ||
question$, |
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.
question$
is possibly undefined, should you tweaks the type QueryAssistContextValue
to question$?: BehaviorSubject<string>;
?
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.
Actually, question$
is always created, it could be undefined because for QueryAssistBanner
, it doesn't use this variable, so I think we don't need to pass question$
to it thus it's undefined
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@ashwin-pc FYI, saw you add 2.17.1 tag, since this is new feature which is not supposed to be included in 2.17.1, so removed that tag from it, let me know if you have any concern, thanks:) |
* Add data summary panel in discover Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Fix UTs Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Add changelog Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Fix UTs Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Address comments Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Address comments Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Address comments Signed-off-by: Liyun Xiu <xiliyun@amazon.com> --------- Signed-off-by: Liyun Xiu <xiliyun@amazon.com> (cherry picked from commit 8d52134) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add data summary panel in discover * Fix UTs * Add changelog * Fix UTs * Address comments * Address comments * Address comments --------- (cherry picked from commit 8d52134) Signed-off-by: Liyun Xiu <xiliyun@amazon.com> 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>
* Add data summary panel in discover Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Fix UTs Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Add changelog Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Fix UTs Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Address comments Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Address comments Signed-off-by: Liyun Xiu <xiliyun@amazon.com> * Address comments Signed-off-by: Liyun Xiu <xiliyun@amazon.com> --------- Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
…rch-project#8264) * Add data summary panel in discover * Fix UTs * Add changelog * Fix UTs * Address comments * Address comments * Address comments --------- (cherry picked from commit 8d52134) Signed-off-by: Liyun Xiu <xiliyun@amazon.com> 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>
Description
UI:
They are part of query assist feature, so only when query assist is available, they could display.
Changes
Data plugin
getSearchBarButton
inQueryEditorExtensionConfig
so that different query editor can display their customized button on search bar.query_enhancements plugin
Issues Resolved
#8177
Screenshot
Testing the changes
Step 1. enable this summary feature in opensearch_dashboards.yml
Step 2. Enable query enhancement from dashboard
OSD left sidebar -> management -> Dashboards Managements -> Advanced settings -> Enable query enhancements -> On
Step 3. configure OpenSearch ML agents
query assist agent
Follow #2 form this doc to configure query assist agent to enable query assist feature
summary agent
Step 4. Navigate to Discover page
Changelog
Add a data summary panel on discover page under query assist function.
Check List
yarn test:jest
yarn test:jest_integration