-
Notifications
You must be signed in to change notification settings - Fork 715
fix: Logs cancel query issue #4375
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
Conversation
WalkthroughThe changes primarily enhance the functionality and error handling of the logs-related components in the application. Key modifications include improved control flow in data processing, the introduction of asynchronous loading for components, and adjustments to column order handling in the search results. These updates aim to streamline user interactions and ensure more robust error management throughout the log search functionalities. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
c11007d to
d60c9f9
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- web/src/composables/useLogs.ts (5 hunks)
- web/src/plugins/logs/JsonPreview.vue (4 hunks)
- web/src/plugins/logs/SearchResult.vue (1 hunks)
- web/src/plugins/logs/TenstackTable.vue (2 hunks)
Additional comments not posted (14)
web/src/plugins/logs/JsonPreview.vue (2)
234-234: LGTM!The import of
defineAsyncComponentis correct and necessary for defining asynchronous components.The code changes are approved.
268-270: LGTM!Defining
QueryEditoras an asynchronous component usingdefineAsyncComponentimproves the loading behavior by loading it only when needed.The code changes are approved.
web/src/plugins/logs/TenstackTable.vue (2)
476-476: LGTM!Disabling sorting by default in
useVueTableconfiguration might be intentional to improve performance or user experience.The code changes are approved.
688-688: LGTM!Updating the background color of the
.resizerclass to$primaryensures consistency in the UI.The code changes are approved.
web/src/plugins/logs/SearchResult.vue (7)
259-259: LGTM!Introducing
updatedColOrderenhances the flexibility of the column order handling by allowing for the dynamic adjustment of the order based on the presence of default columns.The code changes are approved.
261-262: LGTM!Removing the timestamp column from
updatedColOrderensures that it is not included in the saved column order, which might be a requirement for the application.The code changes are approved.
264-265: LGTM!Removing the "source" column from
updatedColOrderif its resizing is disabled ensures that it is not included in the saved column order, which might be a requirement for the application.The code changes are approved.
264-264: LGTM!Finding the index of the "source" column using
updatedColOrder.indexOf("source")is necessary to determine if it is present inupdatedColOrder.The code changes are approved.
266-267: LGTM!Finding the index of the "source" column in the columns array using
columns.findIndex((col: any) => col.name === "source")is necessary to determine if it is present in the columns array.The code changes are approved.
271-272: LGTM!Checking the resizing property of the "source" column using
columns[sourceColIndex].enableResizing === falseensures that it is removed fromupdatedColOrderif its resizing is disabled.The code changes are approved.
277-278: LGTM!Updating the column order using
this.searchObj.data.resultGrid.colOrder[this.searchObj.data.stream.selectedStream] = [...updatedColOrder]ensures that the column order is updated with the modifiedupdatedColOrder.The code changes are approved.
web/src/composables/useLogs.ts (3)
2334-2342: LGTM!The function
getHistogramQueryDatahas been simplified to streamline the condition for checking if the operation was cancelled. The changes enhance the clarity and efficiency of the code.The code changes are approved.
Line range hint
3004-3026: LGTM!The function
updateGridColumnshas been simplified to handle theresultGrid.columnsarray. The changes enhance the clarity and efficiency of the code.The code changes are approved.
1705-1707: LGTM! But verify the function usage.The function
generateHistogramSkeletonis correctly implemented. However, ensure that the function is called in the correct context after reversing thepartitionsarray.The code changes are approved.
Run the following script to verify the function usage:
Verification successful
Function
generateHistogramSkeletonis used correctly.The function
generateHistogramSkeletonis implemented and used appropriately within theweb/src/composables/useLogs.tsfile. Its usage aligns with the expected behavior of preparing the histogram skeleton before further processing.
- The function is awaited before iterating over
partitions.- It is also called before generating histogram data.
No issues were found with its usage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `generateHistogramSkeleton`. # Test: Search for the function usage. Expect: Only occurrences of the new context. rg --type javascript -A 5 $'generateHistogramSkeleton'Length of output: 88
Script:
#!/bin/bash # Description: Verify all function calls to `generateHistogramSkeleton`. # Test: Search for the function usage. Expect: Only occurrences of the new context. rg --type typescript -A 5 $'generateHistogramSkeleton'Length of output: 88
Script:
#!/bin/bash # Description: Verify all function calls to `generateHistogramSkeleton`. # Test: Search for the function usage. Expect: Only occurrences of the new context. rg -A 5 $'generateHistogramSkeleton'Length of output: 1288
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- web/src/composables/useLogs.ts (5 hunks)
- web/src/plugins/logs/JsonPreview.vue (4 hunks)
- web/src/plugins/logs/SearchResult.vue (1 hunks)
- web/src/plugins/logs/TenstackTable.vue (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- web/src/composables/useLogs.ts
- web/src/plugins/logs/JsonPreview.vue
- web/src/plugins/logs/SearchResult.vue
- web/src/plugins/logs/TenstackTable.vue
d60c9f9 to
993137a
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- web/src/composables/useLogs.ts (5 hunks)
- web/src/plugins/logs/JsonPreview.vue (4 hunks)
- web/src/plugins/logs/SearchResult.vue (1 hunks)
- web/src/plugins/logs/TenstackTable.vue (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- web/src/composables/useLogs.ts
- web/src/plugins/logs/JsonPreview.vue
- web/src/plugins/logs/SearchResult.vue
- web/src/plugins/logs/TenstackTable.vue
fix #4344
1. Sometimes user has to click cancel query multiple times for the query to get cancelled
2. Add Column and expand log, click on column header. Now when we expand any row, it does not expands at desired
position
Reverted the previous timestamp column functionality.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores