feat: added kafka - scenario - 4 #6370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 6ec43f3 in 1 minute and 0 seconds
More details
- Looked at
204lines of code in7files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetails/MQTables/getKafkaSpanEval.tsx:32
- Draft comment:
Consider adding error handling for network errors in thegetKafkaSpanEvalfunction to prevent unhandled promise rejections. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The function already includes a try-catch block to handle errors, which is a standard way to manage network errors in asynchronous functions. The comment does not provide specific guidance on what additional error handling is required, making it speculative. The existing error handling seems sufficient for the function's purpose.
The comment might be suggesting a more specific type of error handling that isn't immediately obvious, such as handling specific error codes or retry logic. However, without specific guidance, it's hard to determine the necessity of the comment.
The lack of specificity in the comment makes it difficult to act upon, and the existing error handling seems to cover the general case of network errors. Without clear evidence of a deficiency, the comment appears unnecessary.
The comment should be deleted as it is speculative and the existing error handling seems sufficient. The comment does not provide actionable guidance.
2. frontend/src/pages/MessagingQueues/MQDetails/MQDetails.tsx:142
- Draft comment:
The return statement fordropRateview incheckValidityOfDetailConfigsmight not be correct. Verify if this logic is intended. - Reason this comment was not posted:
Comment did not seem useful.
3. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:181
- Draft comment:
Avoid using inline styles for theCalendarcomponent. Use CSS classes or styled components instead. This applies to other instances in this file as well. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_z7aZbkVPHWSUWV61
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
ahmadshaheer
reviewed
Nov 5, 2024
frontend/src/pages/MessagingQueues/MQDetails/MQTables/getKafkaSpanEval.tsx
Outdated
Show resolved
Hide resolved
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on ba89889 in 52 seconds
More details
- Looked at
319lines of code in7files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetails/MQTables/getKafkaSpanEval.tsx:15
- Draft comment:
Consider re-adding error handling for the axios post request to manage potential errors effectively. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/pages/MessagingQueues/MQDetails/MQTables/getPartitionLatencyDetails.ts:23
- Draft comment:
Consider re-adding error handling for the axios post request to manage potential errors effectively. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/pages/MessagingQueues/MQDetails/MQTables/getPartitionLatencyOverview.ts:14
- Draft comment:
Consider re-adding error handling for the axios post request to manage potential errors effectively. - Reason this comment was not posted:
Marked as duplicate.
4. frontend/src/pages/MessagingQueues/MQDetails/MQTables/getTopicThroughputDetails.ts:16
- Draft comment:
Consider re-adding error handling for the axios post request to manage potential errors effectively. - Reason this comment was not posted:
Marked as duplicate.
5. frontend/src/pages/MessagingQueues/MQDetails/MQTables/getTopicThroughputOverview.ts:15
- Draft comment:
Consider re-adding error handling for the axios post request to manage potential errors effectively. - Reason this comment was not posted:
Marked as duplicate.
6. frontend/src/pages/MessagingQueues/MQDetails/MQTables/MQTables.tsx:1
- Draft comment:
Avoid using thecomponent/index.tsxfile structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is also applicable in other files likegetKafkaSpanEval.tsx,getPartitionLatencyDetails.ts, etc. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_hu3GW47unp9OWs0H
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
frontend/src/pages/MessagingQueues/MQDetails/MQTables/getConsumerLagDetails.ts
Show resolved
Hide resolved
rahulkeswani101
approved these changes
Nov 6, 2024
ahmadshaheer
approved these changes
Nov 6, 2024
There was a problem hiding this comment.
Thanks for making the changes, @SagarRajput-7 🙏 🙂.
assuming that you have tested the error states.
Yes |
YounixM
approved these changes
Nov 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Added scenario - 4 changes
Related Issues / PR's
Screenshots
Screen.Recording.2024-11-05.at.7.18.35.PM.mov
Affected Areas and Manually Tested Areas
Important
Add 'dropRate' view to Kafka messaging queues with new API and UI updates.
MQDetailPage.tsx, replacing 'consumerLatency'.MessagingQueuesDetailsto exclude 'dropRate' from certain views.getKafkaSpanEvalingetKafkaSpanEval.tsxfor 'dropRate' API calls.getKafkaSpanEvalfor evaluating Kafka spans.getConsumerLagDetails.tsto include optionalevalTime.MessagingQueueOverview.tsxto handle 'dropRate' view.MessagingQueues.tsxto reflect 'dropRate' in UI.MessagingQueuesViewTypeinMessagingQueuesUtils.ts.This description was created by
for ba89889. It will automatically update as commits are pushed.