Skip to content
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

chore: restructure Kafka codebase #6611

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

SagarRajput-7
Copy link
Contributor

@SagarRajput-7 SagarRajput-7 commented Dec 11, 2024

Summary

Restructure the code file of Kafka feature

  • Moved apis to API section
  • Components to components

Related Issues / PR's

Screenshots

NA

Affected Areas and Manually Tested Areas


Important

Restructure Kafka codebase by moving files to api/messagingQueues and components directories and updating import paths.

  • File Restructuring:
    • Move getConsumerLagDetails.ts, getKafkaSpanEval.tsx, getPartitionLatencyDetails.ts, getPartitionLatencyOverview.ts, getTopicThroughputDetails.ts, and getTopicThroughputOverview.ts to api/messagingQueues.
    • Move AttributeCheckList.tsx, MessagingQueueHealthCheck.styles.scss, and MessagingQueueHealthCheck.tsx to components/MessagingQueueHealthCheck.
    • Move MQCommon.styles.scss and MQCommon.tsx to components/MessagingQueues.
  • Imports Update:
    • Update import paths in getKafkaSpanEval.tsx, AttributeCheckList.tsx, MessagingQueueHealthCheck.tsx, ConnectionStatus.tsx, DropRateView.tsx, MQTables.tsx, MessagingQueueOverview.tsx, MQConfigOptions.tsx, MessagingQueues.tsx, and MessagingQueuesUtils.ts to reflect new file locations.

This description was created by Ellipsis for 29c93c7. It will automatically update as commits are pushed.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the chore label Dec 11, 2024
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 8d304e4 in 56 seconds

More details
  • Looked at 268 lines of code in 18 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. frontend/src/api/messagingQueues/getKafkaSpanEval.tsx:2
  • Draft comment:
    Duplicate import of DropRateAPIResponse. 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 of MessagingQueueHealthCheckService. 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 of MessagingQueueHealthCheckService. 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 of MessagingQueueHealthCheck. 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 of MessagingQueueHealthCheck. 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.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@pranay01 pranay01 deleted the branch main December 19, 2024 13:03
@pranay01 pranay01 closed this Dec 19, 2024
@SagarRajput-7 SagarRajput-7 reopened this Dec 21, 2024
@SagarRajput-7 SagarRajput-7 changed the base branch from develop to main December 21, 2024 06:41
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 7411eb7 in 1 minute and 4 seconds

More details
  • Looked at 230 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 10 drafted comments based on config settings.
1. frontend/src/api/messagingQueues/getKafkaSpanEval.tsx:2
  • Draft comment:
    Remove the unused import statement for DropRateAPIResponse from 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 for MessagingQueueServicePayload from 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
ahmadshaheer previously approved these changes Dec 22, 2024
@ahmadshaheer
Copy link
Collaborator

@SagarRajput-7, the build is failing. can you please check.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 29c93c7 in 30 seconds

More details
  • Looked at 31 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MQDetails/MQTables/MQTableUtils.tsx:1
  • Draft comment:
    The import path for MessagingQueuesPayloadProps is 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 the component/index.tsx file 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants