[CF-4208] Add --name/--status filtering to on-prem application list - #3428
Conversation
|
❌ Error getting contributor login(s). |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds application-list filtering support for Flink on-prem by wiring CLI flags into the CMF “filter” query parameter and updating the on-prem test server + integration fixtures to exercise name/status filtering behavior.
Changes:
- Add
--name(wildcard suffix supported) and--statusflags toflink application list, and build a CMFfilterquery from them. - Extend the CMF REST client
ListApplicationsto accept an optional filter and include it in requests. - Update the on-prem test server to emulate CMF-side filtering and add new integration test cases + golden outputs.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test-server/flink_onprem_handler.go | Emulates CMF filter behavior for on-prem application listing. |
| pkg/flink/cmf_rest_client.go | Adds filter support to the REST client listing call. |
| internal/flink/command_application_list.go | Introduces --name/--status flags and composes the CMF filter query. |
| internal/flink/command_application_list_test.go | Unit-tests the filter composition helper. |
| test/flink_onprem_test.go | Adds integration scenarios for list filtering. |
| test/fixtures/output/flink/application/*.golden | Golden outputs for the new filtering scenarios and updated help text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| switch key { | ||
| case "name": | ||
| name, _ := app.Metadata["name"].(string) | ||
| if prefix, isWildcard := strings.CutSuffix(value, "*"); isWildcard { | ||
| return strings.HasPrefix(name, prefix) | ||
| } | ||
| return name == value | ||
| case "state": | ||
| if app.Status == nil { | ||
| return false | ||
| } | ||
| jobStatus, ok := (*app.Status)["jobStatus"].(map[string]interface{}) | ||
| if !ok { | ||
| return false | ||
| } | ||
| state, _ := jobStatus["state"].(string) | ||
| return strings.EqualFold(state, value) | ||
| default: | ||
| return true | ||
| } |
| for _, expr := range strings.Split(filter, ",") { | ||
| key, value, found := strings.Cut(expr, "=") | ||
| if !found { | ||
| continue | ||
| } |
Steven Gagniere (sgagniere)
left a comment
There was a problem hiding this comment.
Hi, I have a few comments:
|
|
||
| cmd.Flags().String("environment", "", "Name of the Flink environment.") | ||
| cmd.Flags().String("name", "", `Filter the Flink applications by name. Supports wildcards, for example "my-app*".`) | ||
| cmd.Flags().String("status", "", "Filter the Flink applications by status.") |
There was a problem hiding this comment.
I think we should name this state to match the filter key.
| // "state=" filter, per the cmf-sdk-go GetApplications filter documentation. Unknown values are | ||
| // still forwarded (the server returns no matches rather than erroring); this list only drives | ||
| // the advisory --status warning. | ||
| var allowedApplicationStatuses = []string{"RUNNING", "FINISHED", "FAILED", "CANCELED", "RECONCILING", "COMPLETED", "UNKNOWN"} |
There was a problem hiding this comment.
Just to confirm: are all of these statuses accepted filter arguments? The spec's description for filterParam implies that only RUNNING or FAILED are valid; so the description may be out of date for the spec.
52878a9 to
f5c4f57
Compare
Stacked on the --page-size PR. Add server-side filtering to `flink application list` via --name (supports a "*" suffix wildcard) and --status, composed into the CMF applications "filter" query (name=<value>,state=<value>). ListApplications gains a filter parameter that is applied via the SDK's .Filter(...) before pagination. An unrecognized --status prints a [WARN] to stderr but still queries, since the CMF server treats an unknown state as a no-match rather than an error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
f5c4f57 to
7a68092
Compare
Release Notes
New Features
--nameand--statusfilters to the on-prem (Confluent Platform / CMF)confluent flink application listcommand, to filter large environments server-side instead of "list everything then grep".Checklist
Whatsection below whether this PR applies to Confluent Cloud, Confluent Platform, or both.Test & Reviewsection below.Blast Radiussection below.What
Confluent Platform (CMF on-prem) only — filtering half of CF-4208; Confluent Cloud
flinkcommands are untouched.flink application listhad no filtering, so at scale the only pattern was "list everything then grep" (CF-4202). This adds server-side filtering:--name— by application name; supports a trailing*wildcard (e.g.--name my-app*).--status— by Flink job state (RUNNING,FINISHED,FAILED,CANCELED,RECONCILING,COMPLETED,UNKNOWN).Both compose into CMF's single generic
filterquery asname=<value>,state=<value>;ListApplicationsgains afilterargument applied before pagination. An unknown--statusprints a[WARN]to stderr but still queries, since CMF treats an unknown state as a no-match (matchingstatement list --status).Blast Radius
application list; with neither flag set, behavior is unchanged (nofilterparam). NoCmfClientInterface/mock change. Non-breaking, easy to revert.References
--page-sizeto on-prem Flink list commands #3424.Test & Review
TestBuildApplicationFilter: name / wildcard / status / combined composition (name=a*,state=RUNNING).--nameexact + wildcard,--statusmatch / no-match / invalid (asserts[WARN]+ empty), combined--name+--status; help golden regenerated.make lintclean.🤖 Generated with Claude Code