HYPERFLEET-299 - docs: Add search and filtering documentation#63
HYPERFLEET-299 - docs: Add search and filtering documentation#63Mischulee wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request consolidates and reorganizes API search and filtering documentation across multiple files. The changes fix incorrect label references in code examples (labels.env → labels.environment), create a new comprehensive search.md guide documenting Tree Search Language (TSL) syntax and operators, and refactor README.md and api-resources.md to defer detailed search documentation to the new dedicated guide while maintaining essential cross-references. No functional changes or new features are introduced. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/search.md`:
- Line 100: The docs incorrectly list `Unknown` as a valid status for resource
conditions; update the search documentation text so Condition types remain
PascalCase (`Ready`, `Available`, `Progressing`) but the allowed status for
synthesized resource conditions uses only `True` and `False` to match
`ResourceConditionStatus`, and note that `Unknown` is valid only for
`AdapterConditionStatus` (adapter status reports). Replace any examples or lists
that include `Unknown` for resource conditions with `True`/`False` and add a
brief clarifying sentence that `Unknown` applies only to
`AdapterConditionStatus`.
- Line 100: Add the missing condition type "Progressing" to the documented list
of condition types (alongside "Available", "Ready", "Applied", and "Health") in
the API resources documentation where condition types and their allowed statuses
are described; ensure "Progressing" is listed in PascalCase and mention that
status values remain "True", "False", or "Unknown" so the docs match the
code/tests that reference Progressing.
- Around line 24-26: The document currently mixes operator casing; update all
instances of logical operators in the Complex Queries section and examples to
match the operator table by using lowercase `and`, `or`, and `not` consistently
(replace any `AND`/`OR`/`NOT` usages in the "Complex Queries" examples and
related examples showing query expressions with the lowercase forms `and`, `or`,
`not`) so the operator table and examples are standardized.
12c6de1 to
fdd634d
Compare
fdd634d to
e4982aa
Compare
e4982aa to
8370e3e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 253-254: The Makefile contains duplicated environment assignments
for HYPERFLEET_CLUSTER_ADAPTERS and HYPERFLEET_NODEPOOL_ADAPTERS with trailing
backslashes that are not followed by commands, producing dangling shell
continuations that break targets like run and run-no-auth; remove the duplicate
post-serve lines or delete the trailing backslashes after the duplicated
HYPERFLEET_CLUSTER_ADAPTERS and HYPERFLEET_NODEPOOL_ADAPTERS entries so each
environment assignment is either part of a continued recipe line or stands alone
without a backslash (check the blocks around the serve target and the .PHONY:
run declaration to ensure no dangling continuations remain).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
MakefileREADME.mddocs/api-resources.mddocs/search.md
✅ Files skipped from review due to trivial changes (1)
- docs/search.md
8370e3e to
627cc64
Compare
627cc64 to
970203d
Compare
|
|
||
| Query resources by status conditions: `status.conditions.<Type>='<Status>'` | ||
|
|
||
| Condition types must be PascalCase (`Ready`, `Available`) and status must be `True` or `False` for resource conditions. |
There was a problem hiding this comment.
The code in conditionsNodeConverter explicitly restricts status.conditions. fields to only the = operator:
if n.Func != tsl.EqOp {
return nil, errors.BadRequest("only equality operator (=) is supported for condition queries")
}This means:
- status.conditions.Ready!='False' → returns a 400 error
- not status.conditions.Ready='True' → silently returns empty results (the condition extraction replaces the node with 1 = 1, so not (1 = 1) is always false)
But the document mentions !=, <, <=, >, >=, not, in as supported operators.
The "Status Condition Queries" section should mention that
status.conditions. fields only support the = operator. Other operators
(!=, <, >, in, not, etc.) will either return a 400 error or silently produce
incorrect results.
Suggested addition after line 100:
| Condition types must be PascalCase (`Ready`, `Available`) and status must be `True` or `False` for resource conditions. | |
| Condition types must be PascalCase (`Ready`, `Available`) and status must be `True` or `False` for resource conditions. | |
| **Note:** Only the `=` operator is supported for condition queries. Other operators (`!=`, `<`, `>`, `in`, etc.) will return an error. Use `not` with conditions cautiously — `not status.conditions.Ready='True'` does not work as expected. |
Anyway, besides the note, I've opened a jira ticket to fix the bug: https://issues.redhat.com/browse/HYPERFLEET-709
HYPERFLEET-709 will also update this document when it gets resolved.
https://issues.redhat.com/browse/HYPERFLEET-299
Summary by CodeRabbit