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

Merged
merged 4 commits into from
Jan 20, 2025
Merged

chore: correct order within page result #6847

merged 4 commits into from
Jan 20, 2025

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.

@srikanthccv srikanthccv enabled auto-merge (squash) January 20, 2025 13:30
@srikanthccv srikanthccv merged commit 8954186 into main Jan 20, 2025
16 checks passed
@srikanthccv srikanthccv deleted the within-page-sort branch January 20, 2025 13:38
grandwizard28 added a commit that referenced this pull request Jan 20, 2025
feat: add search span scope in the list view tab to add the scope at per Query level (#6810)

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

* feat: add search span scope in the list view tab to add the scope at per query level

chore(install-script): install.sh script improvements (#6857)

- support for docker compose plugin
- check for occupied port when installing SigNoz for the first time
- remove unused code

Signed-off-by: Prashant Shahi <prashant@signoz.io>

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

2 participants