-
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 right panel graphs in overview page #6944
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
9d30924
to
13d962b
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.
❌ Changes requested. Reviewed everything up to 13d962b in 2 minutes and 15 seconds
More details
- Looked at
1609
lines of code in19
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. frontend/src/AppRoutes/routes.ts:412
- Draft comment:
The route key has been changed from 'MESSAGING_QUEUES_CELERY_OVERVIEW' to 'MESSAGING_QUEUES_OVERVIEW'. Ensure this change is intentional and consistent across the codebase. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/components/CeleryOverview/CeleryOverviewTable/CeleryOverviewTable.tsx:70
- Draft comment:
Consider using more specific types instead ofany
for better type safety and maintainability. - Reason this comment was not posted:
Confidence changes required:50%
The PR introduces a new search functionality in the table columns. This is a good addition for usability, but the implementation could be optimized by avoiding the use ofany
type and ensuring type safety.
3. frontend/src/components/CeleryTask/useNavigateToTraces.ts:11
- Draft comment:
Ensure that the filters and time range are correctly formatted and set when usinguseNavigateToTraces
. - Reason this comment was not posted:
Confidence changes required:50%
TheuseNavigateToTraces
hook is used to navigate to traces with filters. The implementation seems correct, but it's important to ensure that the filters are correctly formatted and that the time range is accurately set.
4. frontend/src/container/MetricsApplication/Tabs/util.ts:107
- Draft comment:
Remove or replace console logging with a proper logging mechanism for production code. - Reason this comment was not posted:
Confidence changes required:50%
TheonGraphClickHandler
function logs the parameters to the console. This might be useful for debugging but should be removed or replaced with a proper logging mechanism in production code.
5. frontend/src/pages/Celery/CeleryOverview/CeleryOverviewDetail/ValueInfo.tsx:92
- Draft comment:
Consider making the thresholds for color coding configurable or document them clearly. - Reason this comment was not posted:
Confidence changes required:40%
ThegetColorBasedOnValue
function uses hardcoded thresholds for color coding. It might be beneficial to make these thresholds configurable or document them clearly.
Workflow ID: wflow_2lljt8yjnD8VIWJD
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/Celery/CeleryOverview/CeleryOverviewDetail/CeleryOverviewDetails.styles.scss
Show resolved
Hide resolved
frontend/src/components/CeleryOverview/CeleryOverviewTable/CeleryOverviewTable.tsx
Show resolved
Hide resolved
frontend/src/components/CeleryOverview/CeleryOverviewTable/CeleryOverviewTable.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> |
7383302
to
508c54d
Compare
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> |
Summary
Related Issues / PR's
Screenshots
Screen.Recording.2025-01-28.at.8.25.05.AM.mov
Affected Areas and Manually Tested Areas
Important
Added right panel graphs to the messaging queues overview page with new components and updated styles and routes.
CeleryOverview.tsx
.CeleryOverviewDetails
component for detailed view with graphs inCeleryOverviewDetails.tsx
.OverviewRightPanelGraph
andValueInfo
components for displaying graphs and metrics.CeleryOverviewTable
to handle row clicks and display details.useNavigateToTraces
hook for navigation to traces.CeleryTaskGraph
with new graph configurations.CeleryOverviewConfigOptions.styles.scss
,CeleryOverviewDetails.styles.scss
, andValueInfo.styles.scss
for new UI elements.ROUTES.MESSAGING_QUEUES_CELERY_OVERVIEW
toROUTES.MESSAGING_QUEUES_OVERVIEW
.QueryParams
androuteConfig
to include new parameters and routes.This description was created by for 7383302. It will automatically update as commits are pushed.