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: correct order within page result #6847

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

srikanthccv
Copy link
Member

@srikanthccv srikanthccv commented Jan 18, 2025

Summary

Part of #5373


Important

Add sorting functionality to various list responses in the inframetrics package, allowing records to be sorted based on specified OrderBy criteria.

  • Behavior:
    • Adds SortBy method to ClusterListResponse, DaemonSetListResponse, DeploymentListResponse, JobListResponse, NamespaceListResponse, NodeListResponse, PodListResponse, StatefulSetListResponse, and VolumeListResponse in infra.go.
    • Sorts records based on OrderBy criteria, defaulting to descending order.
    • Reverses order if OrderBy.Order is v3.DirectionAsc.
  • Functions:
    • Calls SortBy in GetClusterList() in clusters.go, GetDaemonSetList() in daemonsets.go, GetDeploymentList() in deployments.go, GetJobList() in jobs.go, GetNamespaceList() in namespaces.go, GetNodeList() in nodes.go, GetPodList() in pods.go, GetPvcList() in pvcs.go, and GetStatefulSetList() in statefulsets.go.

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

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 b07aa18 in 1 minute and 21 seconds

More details
  • Looked at 539 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 9 drafted comments based on config settings.
1. pkg/query-service/app/inframetrics/clusters.go:341
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. pkg/query-service/app/inframetrics/daemonsets.go:443
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
3. pkg/query-service/app/inframetrics/deployments.go:443
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
4. pkg/query-service/app/inframetrics/jobs.go:497
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
5. pkg/query-service/app/inframetrics/namespaces.go:344
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
6. pkg/query-service/app/inframetrics/nodes.go:357
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
7. pkg/query-service/app/inframetrics/pods.go:407
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
8. pkg/query-service/app/inframetrics/pvcs.go:377
  • Draft comment:
    Check if req.OrderBy is nil before calling SortBy to avoid potential nil pointer dereference.
  • Reason this comment was not posted:
    Marked as duplicate.
9. pkg/query-service/model/infra.go:117
  • Draft comment:
    Avoid adding non-ClickHouse related functions to the ClickHouseReader interface. The SortBy method is correctly placed in the model package.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_KSzGVudvxgEeBxrB


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.

1 participant