Handle exclude predicate#4439
Open
balanza wants to merge 5 commits into
Open
Conversation
balanza
force-pushed
the
poc-check-predicate
branch
4 times, most recently
from
024bc29 to
9f3c3a6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the checks execution/request pipeline and the Execution Results UI to correctly represent hosts that are excluded by a check’s exclude predicate (and to support predicates that rely on host attributes like is_majority_maker).
Changes:
- Add
is_majority_makeras a target attribute when requesting cluster checks executions (and plumb this attribute from cluster details to host targets). - Surface
excluded_by_policyagent results in the UI as a distinct “Excluded by policy” row and show a dedicated detail view including the exclusion predicate. - Update contracts dependency ref to include the updated protobuf schema (e.g., target
attributes).
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/trento/infrastructure/checks/checks_test.exs | Adds coverage for target attributes and is_majority_maker propagation in execution requests. |
| test/trento/clusters_test.exs | Verifies is_majority_maker is set in target attributes for HANA scale-up cluster nodes. |
| lib/trento/infrastructure/checks/checks.ex | Adds protobuf target attributes (including is_majority_maker) to ExecutionRequested targets. |
| lib/trento/clusters.ex | Computes majority-maker hostnames from cluster details and passes is_majority_maker into checks target data. |
| assets/js/lib/model/index.js | Adds the EXCLUDED_BY_POLICY status constant. |
| assets/js/pages/ExecutionResults/checksUtils.js | Adds isAgentExcluded helper based on EXCLUDED_BY_POLICY. |
| assets/js/pages/ExecutionResults/CheckResultOutline.jsx | Renders excluded hosts as “Excluded by policy” rows and avoids treating them as evaluated results. |
| assets/js/pages/ExecutionResults/TargetResult.jsx | Adds styling support for excluded rows (isExcluded). |
| assets/js/pages/ExecutionResults/CheckResultOutline.test.jsx | Adds test coverage for the excluded-by-policy row rendering. |
| assets/js/pages/ExecutionResults/CheckResultDetail/CheckResultDetail.jsx | Adds excluded-by-policy detail rendering including the exclusion predicate expression. |
| assets/js/pages/ExecutionResults/CheckResultDetail/CheckResultDetail.test.jsx | Adds test coverage for excluded-by-policy details. |
| assets/js/lib/test-utils/factories/executions.js | Adds an agentCheckExcludedFactory for excluded agent results in UI tests. |
| mix.exs | Bumps trento_contracts git ref. |
| mix.lock | Locks trento_contracts to the updated git ref. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -53,8 +55,12 @@ defmodule Trento.Infrastructure.Checks do | |||
| targets: | |||
balanza
force-pushed
the
poc-check-predicate
branch
from
9f3c3a6 to
793f827
Compare
The Target protobuf field was renamed from host_data to attributes in the contracts submodule. Update web's checks engine and tests to use the new field name. Update mix.exs/mix.lock to point to contracts commit f3c137c which introduces the new attributes field.
Wanda returns excluded (host, check) pairs with status "excluded_by_policy" and the exclude_expression. Surface them as a distinct, neutral "Excluded by policy" row in the check results outline (instead of a red 0/N expectations failure) and show the exclusion predicate in the check detail panel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "pass host data to checks" commit accidentally removed the maybe_put_error_details clauses and map_request_failed_error helper from QueueEventParser (a rebase artifact), breaking QueueEventParserTest. Restore them to match main.
balanza
force-pushed
the
poc-check-predicate
branch
from
c2a8062 to
645840d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following RFC trento-project/docs#188
This PR extends the check execution flow to pass per-host metadata (starting with is_majority_maker) as target attributes, enabling check policies to reference host properties. It also updates the execution results UI to surface hosts that were excluded by a check's exclude predicate.