resource/cmd: support comparison operators - #418
Conversation
jedevc
left a comment
jedevc
left a comment
There was a problem hiding this comment.
Looooks neat! Sorry for the delay again (I was on holiday 🌴).
| // fieldAdaptor implements filters.Adaptor over a resource's fields, using | ||
| // resource.GetFieldByPath to resolve field paths, and exposing raw typed | ||
| // values so comparison operators (>, <, >=, <=) can work against native | ||
| // types (e.g. memory>128mib, created_at>2026-01-01). | ||
| type fieldAdaptor struct { |
There was a problem hiding this comment.
Let's move all this out into filter.go since it's now more complex.
| // newFieldAdaptor creates a filters.Adaptor that can traverse resource fields. | ||
| // It handles both structured fields (with subfields) and slice values (like []string tags). |
| if _, _, _, _, ok := a.resolve(full); !ok { | ||
| return nil, false | ||
| } | ||
| return &fieldAdaptor{fields: a.fields, prefix: full}, true |
There was a problem hiding this comment.
Instead of storing the prefix, couldn't we store the whole field?
| return full | ||
| } | ||
|
|
||
| func (a *fieldAdaptor) resolve(path []string) (field *resource.Field, entries []string, sliceVal string, sliceOK bool, ok bool) { |
There was a problem hiding this comment.
We should return a whole fieldAdaptor here I think. This would simplify the code, and avoid so many return values.
Then we also don't need to keep calling resolve over and over, and means we should be able to avoid storing the prefix.
| return value.Compare(field.Value, parsed), true | ||
| } | ||
|
|
||
| func hasOrdering(v any) bool { |
There was a problem hiding this comment.
Hm, I think this should probably be opt-in not opt-out.
E.g. currently this would include arrays/structs/etc. Which aren't ordered.
| parsed, err := value.ParseNew([]string{other}, field.Value) | ||
| if err != nil { | ||
| return 0, false | ||
| } | ||
| return value.Compare(field.Value, parsed), true |
There was a problem hiding this comment.
Hm, worth checking to see how this handles comparing a float field to an integer result - e.g. 1.2<5. Or something like that (something to check works in the tests - it should work as is).
There was a problem hiding this comment.
Pull request overview
Adds typed comparison support for resource filters, addressing #374.
Changes:
- Implements
>,<,>=, and<=field comparisons. - Preserves nested and slice field traversal.
- Refreshes related dependencies and removes obsolete replacements.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/resource/cmd/cmd.go |
Adds the typed filter adaptor and comparison logic. |
go.mod |
Updates dependencies and removes replacements. |
go.sum |
Records updated dependency checksums. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func newFieldAdaptor(fields []resource.Field) filters.Adaptor { | ||
| return &fieldAdaptor{fields: fields} |
| parsed, err := value.ParseNew([]string{other}, field.Value) | ||
| if err != nil { | ||
| return 0, false | ||
| } | ||
| return value.Compare(field.Value, parsed), true |
| // fieldAdaptor implements filters.Adaptor over a resource's fields, using | ||
| // resource.GetFieldByPath to resolve field paths, and exposing raw typed | ||
| // values so comparison operators (>, <, >=, <=) can work against native | ||
| // types (e.g. memory>128mib, created_at>2026-01-01). |
345a785 to
5fca19b
Compare
Signed-off-by: Omkar Ugalmugle <omkarugalmugle285@gmail.com>
5fca19b to
a2acf83
Compare
adds
>,<,>=,<=support in--filter(e.g--filter "resources.memory>128mib").Tested manually against live instances, volumes, and services - memory, vcpus, size, limits all work with all four operators.
Closes #374.
Requires unikraft-cloud/x#307.