fix: take limit as input so the no of operations returned is always limited#2095
Conversation
JivusAyrus
requested review from
Aenimus,
StarpTech,
jensneuse and
wilsonrivera
as code owners
|
Warning Rate limit exceeded@JivusAyrus has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughA new optional integer field limit was added to the GetOperationsRequest message in both the protobuf definition and its TypeScript implementation. The backend logic was updated to accept and use this limit, defaulting to 100 if unspecified, and simplified date range handling by removing feature flag checks and dynamic retention logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Router image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
proto/wg/cosmo/platform/v1/platform.proto (1)
2781-2785: Parameterlimitlooks fine, but align with existing pagination conventionsMost request messages (e.g.,
GetFederatedGraphsRequest,GetSubgraphsRequest) expose bothlimitandoffsetas plainint32(notoptional). Introducinglimithere asoptionaldiverges from that pattern and still leaves callers unable to page through large result sets.Consider the more consistent schema below:
optional string clientName = 3; - optional int32 limit = 4; + int32 limit = 4; // keep semantic parity with other requests + int32 offset = 5; // enable paging through large operation listsThis keeps field‐number compatibility (4 is already new) while giving consumers the same UX they already have elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (4)
connect/src/wg/cosmo/platform/v1/platform_pb.ts(2 hunks)controlplane/src/core/bufservices/analytics/getOperations.ts(2 hunks)controlplane/src/core/repositories/analytics/MetricsRepository.ts(2 hunks)proto/wg/cosmo/platform/v1/platform.proto(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the cosmo codebase, database transactions are typically managed at the service layer (e.g., in bu...
Learnt from: wilsonrivera
PR: wundergraph/cosmo#1919
File: controlplane/src/core/repositories/OrganizationGroupRepository.ts:193-224
Timestamp: 2025-07-01T13:53:54.146Z
Learning: In the Cosmo codebase, database transactions are typically managed at the service layer (e.g., in buf services like deleteOrganizationGroup.ts), where repositories are instantiated with the transaction handle and all operations within those repositories are automatically part of the same transaction.
Applied to files:
controlplane/src/core/bufservices/analytics/getOperations.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
- GitHub Check: image_scan
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./events)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
connect/src/wg/cosmo/platform/v1/platform_pb.ts (2)
21892-21895: LGTM! Properly generated protobuf field declaration.The
limitfield declaration follows correct protobuf TypeScript generation patterns with appropriate JSDoc documentation and optional typing.
21908-21908: LGTM! Correct field descriptor configuration.The field descriptor properly configures the
limitfield with field number 4, INT32 scalar type, and optional flag, maintaining consistency with the protobuf schema.controlplane/src/core/repositories/analytics/MetricsRepository.ts (1)
722-722: LGTM! Method signature properly extended with limit parameter.The addition of the
limitparameter to thegetOperationsmethod signature is clean and follows TypeScript conventions.controlplane/src/core/bufservices/analytics/getOperations.ts (2)
55-55: LGTM! Fixed range simplifies the logic.Using a fixed 7-day range (168 hours) instead of dynamic retention limits simplifies the implementation and removes dependency on feature flags.
70-70: LGTM! Limit parameter properly passed to repository.The limit parameter is correctly passed to the
metricsRepo.getOperationsmethod, completing the integration from service to repository layer.
4 participants
Summary by CodeRabbit
New Features
Improvements
Checklist