Fix lease labels over-filtering#174
Conversation
📝 WalkthroughWalkthroughExtracts matchLabels from Kubernetes-style selectors for server-side lease listing, adds client-side post-filtering for matchExpressions in the CLI, and adds e2e tests covering complex selector and over-filtering scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Client as Client\n(grpc.py)
participant Selector as Selector\n(selectors.py)
participant Server as gRPC Server
participant DB as Lease Store
CLI->>Client: list_leases(selector)
Client->>Selector: extract_match_labels_filter(selector)
Selector-->>Client: matchLabelsFilter (e.g. "k=v,a=b") or null
Client->>Server: ListLeasesRequest(filter=matchLabelsFilter)
Server->>DB: query leases by metadata labels
DB-->>Server: leases (may include items matching matchExpressions)
Server-->>Client: ListLeasesResponse(leases)
Client->>Client: if selector contains matchExpressions\napply client-side filter_by_selector(selector)
Client-->>CLI: filtered leases
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@python/packages/jumpstarter/jumpstarter/client/grpc.py`:
- Line 386: The gRPC call in list_leases strips matchExpressions via
extract_match_labels_filter, so callers that pass a full selector must perform
client-side filtering; update the delete command implementation
(jumpstarter_cli/delete.py inside the delete logic that currently calls
config.list_leases(filter=selector)) to call .filter_by_selector(selector) on
the returned list of leases (or explicitly document why matchExpressions are
irrelevant), ensuring only leases that satisfy the full selector (including
matchExpressions) are processed; reference extract_match_labels_filter,
list_leases, and filter_by_selector when locating the change.
2ffa0b1 to
4b8bb11
Compare
|
hmm.. e2e caught something.. back to draft and investigating. |
4b8bb11 to
71c91a3
Compare
|
ok found the issue, it was the test not the feature. should be good now. let's see the CI 🤞 |
A quick bug fix for server-side over-filtering where a complex selector was being sent to the server for pre-filtering as-is, instead of sending only its matchLabels subset and handling matchExpressions client-side.
also included some e2e tests to cover that.
this is a follow up PR to #168
Summary by CodeRabbit
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.