-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: added celery task feature - with task graphs and details #6840
Conversation
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> |
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.
❌ Changes requested. Reviewed everything up to 593d666 in 1 minute and 18 seconds
More details
- Looked at
1154
lines of code in22
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/components/CeleryTask/CeleryTaskGraph/CeleryTaskGraph.tsx:57
- Draft comment:
Remove the console.log statement to clean up the code for production.
```
- Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/components/CeleryTask/CeleryTaskGraph/CeleryTaskGraph.tsx:47
- Draft comment:
Avoid using inline styles for theCard
component. Use external stylesheets or styled components instead. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/components/CeleryTask/CeleryTaskConfigOptions/CeleryTaskConfigOptions.tsx:66
- Draft comment:
Avoid using inline styles for theButton
component. Use external stylesheets or styled components instead. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_G1Qngh0GJjKTSYsC
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/components/CeleryTask/CeleryTaskDetail/CeleryTaskDetail.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/CeleryTask/CeleryTaskDetail/CeleryTaskDetail.tsx
Show resolved
Hide resolved
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
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.
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 69988a1 in 1 minute and 2 seconds
More details
- Looked at
184
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. frontend/src/components/CeleryTask/CeleryTaskDetail/CeleryTaskDetail.tsx:28
- Draft comment:
Remove the console.log statement to clean up the code.
- Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/components/CeleryTask/CeleryTaskDetail/CeleryTaskDetail.tsx:26
- Draft comment:
Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
3. frontend/src/components/CeleryTask/CeleryTaskGraph/CeleryTaskGraph.style.scss:64
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values to maintain consistency in design and theming. This is also applicable to lines 51, 60, 75, 82, and 92. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_STQTbl1b8q63NvaN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
7a6b6be
to
5516cd5
Compare
Let's remove the attributes, and only show values |
Also, can we use the color theme according to design for different states? |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
84d4b4e
to
bbd651c
Compare
cbd4221
to
e784fba
Compare
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> |
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.
👍 Looks good to me! Reviewed everything up to 10812f5 in 1 minute and 38 seconds
More details
- Looked at
3377
lines of code in45
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. frontend/src/AppRoutes/routes.ts:3
- Draft comment:
import MessagingQueuesMainPage from 'pages/MessagingQueues';
- Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/AppRoutes/routes.ts:28
- Draft comment:
MessagingQueuesMainPage,
- Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/AppRoutes/pageComponents.ts:229
- Draft comment:
export const MessagingQueuesMainPage = Loadable(
() =>
import(/* webpackChunkName: "MessagingQueues" */ 'pages/MessagingQueues/MessagingQueuesMainPage'),
);
- Reason this comment was not posted:
Comment was on unchanged code.
4. frontend/src/pages/Celery/CeleryTask/CeleryTask.styles.scss:17
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values for consistency. This is also applicable in other parts of the stylesheet. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. frontend/src/pages/MessagingQueues/MessagingQueuesMainPage.styles.scss:16
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values for consistency. This is also applicable in other parts of the stylesheet. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_2uxlzWLc4pOPA8x1
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> |
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.
👍 Looks good to me! Incremental review on acb2731 in 37 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/pages/MessagingQueues/MessagingQueues.styles.scss:96
- Draft comment:
Avoid using!important
as it can lead to specificity issues. Consider refactoring the CSS to achieve the desired styling without it. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment raises a valid CSS best practice about avoiding !important. However, sometimes !important is necessary to override third-party styles or deal with specificity conflicts. Without seeing why this !important was added or what styles it's overriding, we can't be certain it's incorrect. The author likely had a specific reason for adding it.
I may be too quick to assume the !important is justified - it could be masking a deeper architectural issue that should be fixed properly. Also, this is a UI-related change which the rules say to generally trust the author on.
While the comment raises a valid point, the rules specifically state to trust the author on UI changes and not to make purely informative comments. The author likely had a reason for using !important here.
Delete this comment since it relates to UI styling which we should trust the author on, and is more of an informative best practice than a clear issue requiring change.
2. frontend/src/pages/MessagingQueues/MessagingQueues.styles.scss:96
- Draft comment:
Avoid using!important
in CSS as it makes styles difficult to override and maintain. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_AxpQKsOPAR2mezGI
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary
Added Celery Task Page
Related Issues / PR's
Screenshots
Screen.Recording.2025-01-27.at.4.25.44.AM.mov
Affected Areas and Manually Tested Areas
Important
Adds Celery task management feature with new components, routes, and styles for task configuration, detail view, and graph display.
CeleryTask
page with task graphs and details inCeleryTask.tsx
.CeleryTaskConfigOptions
,CeleryTaskDetail
, andCeleryTaskGraphGrid
components for task configuration, detail view, and graph display.CeleryUtils.ts
.MESSAGING_QUEUES_CELERY_TASK
inroutes.ts
and integrates it intoAppLayout
andSideNav
.MessagingQueuesMainPage.tsx
to include a tab for Celery tasks.CeleryTask.styles.scss
.MessagingQueues.styles.scss
andMessagingQueuesMainPage.styles.scss
for new layout.GridCard
,PanelWrapper
, andGridTableComponent
to support new graph and table functionalities.permission/index.ts
.This description was created by for acb2731. It will automatically update as commits are pushed.