-
Notifications
You must be signed in to change notification settings - Fork 8
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 configuration section for exporter and delete_after #53
Add configuration section for exporter and delete_after #53
Conversation
51da3cf
to
4463e18
Compare
Do we need to show delete after status in the right side menu? When would the setting be disabled? The value is always 1-180. We could display that instead. |
<h3>Delete After (days)</h3> | ||
</EuiText> | ||
<EuiText size="xs" style={textPadding}> | ||
Delete the Query Insights data after certain days. |
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.
Something like Number of days to retain local index data
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.
Agree, might make more sense to display the number of days. If disabled, it seems like there is no retention period to the user which might not be the case.
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.
Might be good to get UX feedback on this
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.
Updated based on UX feedback
1ca7e47
to
f8415bd
Compare
Signed-off-by: Chenyang Ji <cyji@amazon.com>
Signed-off-by: Chenyang Ji <cyji@amazon.com>
Signed-off-by: Chenyang Ji <cyji@amazon.com>
0649155
to
d071664
Compare
Signed-off-by: Chenyang Ji <cyji@amazon.com>
d071664
to
0da54ee
Compare
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. Nice work!
@@ -23,7 +23,10 @@ describe('Query Group Details Page', () => { | |||
// waiting for the query insights queue to drain | |||
cy.wait(10000); | |||
cy.navigateToOverview(); | |||
cy.get('.euiTableRow').first().find('button').first().trigger('mouseover'); |
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.
Why do we need this?
* Add configuration section for exporter and delete_after Signed-off-by: Chenyang Ji <cyji@amazon.com> * rebase from main and refactor functions Signed-off-by: Chenyang Ji <cyji@amazon.com> * exporter configruation pannels Signed-off-by: Chenyang Ji <cyji@amazon.com> * rebase main to pick up MDS changes Signed-off-by: Chenyang Ji <cyji@amazon.com> --------- Signed-off-by: Chenyang Ji <cyji@amazon.com> (cherry picked from commit 9f955a0) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add configuration section for exporter and delete_after * rebase from main and refactor functions * exporter configruation pannels * rebase main to pick up MDS changes --------- (cherry picked from commit 9f955a0) Signed-off-by: Chenyang Ji <cyji@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
This PR adds configuration panels for
Query Insights Exporters
the newly added configuration endpoint
delete_after_days
Top N indices auto deletion config & functionality query-insights#172enabled
Issues Resolved
#47
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.