chore: restructure Kafka codebase #6611
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.
👍 Looks good to me! Reviewed everything up to 8d304e4 in 56 seconds
More details
- Looked at
268lines of code in18files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. frontend/src/api/messagingQueues/getKafkaSpanEval.tsx:2
- Draft comment:
Duplicate import ofDropRateAPIResponse. Remove the redundant import statement. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/components/MessagingQueueHealthCheck/AttributeCheckList.tsx:22
- Draft comment:
Duplicate import ofMessagingQueueHealthCheckService. Remove the redundant import statement. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. frontend/src/components/MessagingQueueHealthCheck/MessagingQueueHealthCheck.tsx:8
- Draft comment:
Duplicate import ofMessagingQueueHealthCheckService. Remove the redundant import statement. - Reason this comment was not posted:
Marked as duplicate.
4. frontend/src/container/OnboardingContainer/Steps/ConnectionStatus/ConnectionStatus.tsx:9
- Draft comment:
Duplicate import ofMessagingQueueHealthCheck. Remove the redundant import statement. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. frontend/src/pages/MessagingQueues/MessagingQueues.tsx:7
- Draft comment:
Duplicate import ofMessagingQueueHealthCheck. Remove the redundant import statement. - Reason this comment was not posted:
Marked as duplicate.
6. frontend/src/pages/MessagingQueues/MQGraph/MQConfigOptions.tsx:3
- Draft comment:
Ensure that design tokens from '@signozhq/design-tokens' are used consistently throughout the code instead of hardcoded color values. - Reason this comment was not posted:
Confidence changes required:50%
The import statement for colors from '@signozhq/design-tokens' indicates the use of design tokens, which is good. However, I need to ensure that these tokens are used throughout the code instead of hardcoded values.
Workflow ID: wflow_WrewXqiRk1m9I7gK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
SagarRajput-7
force-pushed
the
kafka-api-restructure
branch
from
35c5c48 to
7411eb7
Compare
|
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.
👍 Looks good to me! Incremental review on 7411eb7 in 1 minute and 4 seconds
More details
- Looked at
230lines of code in10files - Skipped
0files when reviewing. - Skipped posting
10drafted comments based on config settings.
1. frontend/src/api/messagingQueues/getKafkaSpanEval.tsx:2
- Draft comment:
Remove the unused import statement forDropRateAPIResponsefrom the old path.
- **Reason this comment was not posted:**
Comment looked like it was already resolved.
</details>
<details>
<summary>2. <code>frontend/src/components/MessagingQueueHealthCheck/AttributeCheckList.tsx:23</code></summary>
- **Draft comment:**
Remove the unused import statement for `MessagingQueueHealthCheckService` from the old path.
```suggestion
- **Reason this comment was not posted:**
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is incorrect because MessagingQueueHealthCheckService is actively used in the code. The import wasn't removed, it was just moved from a relative path to an absolute path. The comment seems to have misinterpreted the diff, seeing the old import being removed but missing that it was replaced with a new import path.
Could there be multiple MessagingQueueHealthCheckService imports that I'm missing? Could the service be imported but not actually needed?
No, looking at the code, the service is clearly used in the handleRedirection function to determine routing paths. The import is necessary for the code to function.
Delete this comment as it is incorrect - the MessagingQueueHealthCheckService import is not unused, it was simply moved to a different import path and is actively used in the code.
</details>
<details>
<summary>3. <code>frontend/src/components/MessagingQueueHealthCheck/MessagingQueueHealthCheck.tsx:8</code></summary>
- **Draft comment:**
Remove the unused import statement for `MessagingQueueHealthCheckService` from the old path.
```suggestion
- **Reason this comment was not posted:**
Marked as duplicate.
</details>
<details>
<summary>4. <code>frontend/src/container/OnboardingContainer/Steps/ConnectionStatus/ConnectionStatus.tsx:9</code></summary>
- **Draft comment:**
Remove the unused import statement for `MessagingQueueHealthCheck` from the old path.
```suggestion
- **Reason this comment was not posted:**
Comment looked like it was already resolved.
</details>
<details>
<summary>5. <code>frontend/src/pages/MessagingQueues/MQDetails/DropRateView/DropRateView.tsx:5</code></summary>
- **Draft comment:**
Remove the unused import statement for `MessagingQueueServicePayload` from the old path.
```suggestion
- **Reason this comment was not posted:**
Comment was not on a valid diff hunk.
</details>
<details>
<summary>6. <code>frontend/src/pages/MessagingQueues/MQDetails/MQTables/MQTables.tsx:7</code></summary>
- **Draft comment:**
Remove the unused import statement for `MessagingQueueServicePayload` from the old path.
```suggestion
```
- Reason this comment was not posted:
Marked as duplicate.
7. frontend/src/pages/MessagingQueues/MQDetails/MessagingQueueOverview.tsx:4
- Draft comment:
Remove the unused import statement forMessagingQueueServicePayloadfrom the old path.
- **Reason this comment was not posted:**
Comment looked like it was already resolved.
</details>
<details>
<summary>8. <code>frontend/src/pages/MessagingQueues/MQGraph/MQConfigOptions.tsx:6</code></summary>
- **Draft comment:**
Remove the unused import statement for `SelectMaxTagPlaceholder` from the old path.
```suggestion
- **Reason this comment was not posted:**
Comment looked like it was already resolved.
</details>
<details>
<summary>9. <code>frontend/src/pages/MessagingQueues/MQGraph/MQConfigOptions.tsx:3</code></summary>
- **Draft comment:**
Use design tokens or predefined color constants instead of hardcoding color values for consistency in theming. This is applicable in other files as well, such as 'ConnectionStatus.tsx'.
- **Reason this comment was not posted:**
Comment was on unchanged code.
</details>
<details>
<summary>10. <code>frontend/src/pages/MessagingQueues/MQGraph/MQConfigOptions.tsx:6</code></summary>
- **Draft comment:**
Use design tokens or predefined color constants instead of hardcoding color values for consistency in theming. This is applicable in other files as well, such as 'ConnectionStatus.tsx'.
- **Reason this comment was not posted:**
Marked as duplicate.
</details>
Workflow ID: <workflowid>`wflow_oTHzMcCxPQPSN3cJ`</workflowid>
</details>
----
You can customize Ellipsis with :+1: / :-1: [feedback](https://docs.ellipsis.dev/review), review rules, user-specific overrides, `quiet` mode, and [more](https://docs.ellipsis.dev/config).
ahmadshaheer
previously approved these changes
Dec 22, 2024
|
@SagarRajput-7, the build is failing. can you please check. |
|
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.
👍 Looks good to me! Incremental review on 29c93c7 in 30 seconds
More details
- Looked at
31lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetails/MQTables/MQTableUtils.tsx:1
- Draft comment:
The import path forMessagingQueuesPayloadPropsis incorrect. It should be updated to reflect the new file location.
import { MessagingQueuesPayloadProps } from 'api/messagingQueues/getConsumerLagDetails';
- Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/api/messagingQueues/getTopicThroughputOverview.ts:7
- 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 applies to other files in the PR as well. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_l6hj9KTk3jQ4OVHH
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
@ahmadshaheer - fixed |
YounixM
approved these changes
Dec 27, 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
Restructure the code file of Kafka feature
componentsRelated Issues / PR's
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Restructure Kafka codebase by moving files to
api/messagingQueuesandcomponentsdirectories and updating import paths.getConsumerLagDetails.ts,getKafkaSpanEval.tsx,getPartitionLatencyDetails.ts,getPartitionLatencyOverview.ts,getTopicThroughputDetails.ts, andgetTopicThroughputOverview.tstoapi/messagingQueues.AttributeCheckList.tsx,MessagingQueueHealthCheck.styles.scss, andMessagingQueueHealthCheck.tsxtocomponents/MessagingQueueHealthCheck.MQCommon.styles.scssandMQCommon.tsxtocomponents/MessagingQueues.getKafkaSpanEval.tsx,AttributeCheckList.tsx,MessagingQueueHealthCheck.tsx,ConnectionStatus.tsx,DropRateView.tsx,MQTables.tsx,MessagingQueueOverview.tsx,MQConfigOptions.tsx,MessagingQueues.tsx, andMessagingQueuesUtils.tsto reflect new file locations.This description was created by
for 29c93c7. It will automatically update as commits are pushed.