Inherit config #244
Inherit config #244
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chentex The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/wip |
chentex
force-pushed
the
inherit-config
branch
from
af1508e to
262b15a
Compare
chentex
force-pushed
the
inherit-config
branch
from
c2b3fa7 to
1a349b7
Compare
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
chentex
force-pushed
the
inherit-config
branch
from
84d0aa8 to
88ed8e1
Compare
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
vishnuchalla
left a comment
vishnuchalla
left a comment
There was a problem hiding this comment.
Nice, change. Thanks for the head start on this.
|
|
||
| **How it works:** | ||
| - The parent configuration file should contain a `metadata` section | ||
| - Metadata from the parent is merged into each test's metadata |
There was a problem hiding this comment.
Can we keep options for global vs test level inheritance? Because we might run into problems considering set operation. For example if we have globally inherited file and only some fields are using in the child test case but not all, then that might be a problem even if the child has its own set of metadata defined.
|
|
||
| ## Configuration Inheritance | ||
|
|
||
| Orion supports configuration inheritance to reduce duplication and improve maintainability. You can inherit metadata from a parent configuration file and load metrics from an external file. |
There was a problem hiding this comment.
Here external might confuse people with remote URL. Maybe in the follow ups, that would actually be cool for other team adoptions.
| - Metrics from the external file are merged with metrics defined in the test | ||
| - **Test-level metrics take precedence** - if a metric with the same `name` and `metricName` exists in both, the test-level metric is used | ||
| - Paths can be relative (to the config file directory) or absolute | ||
| - Metrics are merged: inherited metrics that don't conflict are included, then test-level metrics are added |
There was a problem hiding this comment.
We need to have a flexibility here. Allowing straight (A u B) might not fit for every use case
| ) | ||
|
|
||
| for test in rendered_config["tests"]: | ||
| test.setdefault("version_field", "ocpVersion") |
There was a problem hiding this comment.
We can still stick to setdefault here?
There was a problem hiding this comment.
if the version_field is not set, we need to use a default value for it. I tried removing this but the logic is always expecting a value here. I will investigate if I can do other things.
| return render_template(parent_config_content, env_vars, logger) | ||
|
|
||
|
|
||
| def load_metrics_file(metrics_file: str, |
There was a problem hiding this comment.
This is similar to above function, except for the variable name. We can considering modularizing it?
|
|
||
| # Iterate through config keys and add them, overriding inherited_config values | ||
| # If a key exists in config, skip adding it from inherited_config (config takes precedence) | ||
| for key in config: |
| orion --lookback 15d --since 2026-01-20 --hunter-analyze --config hack/ci-tests/ci-tests-inherits.yaml --metadata-index "orion-integration-test-data*" --benchmark-index "orion-integration-test-metrics*" --es-server=${QE_ES_SERVER} --node-count true --input-vars='{"version": "4.20"}' > ./outputs/results.txt | ||
| EXIT_CODE=$? | ||
|
|
||
| if [ ! $EXIT_CODE -eq 2 ]; then |
There was a problem hiding this comment.
This seems to be redundant in all the tests. Can we modularize it?
|
Just to confirm @vishnuchalla, this would be the expected configuration layering: 
|
3 participants
Type of change
Description
Inherit the config from other yaml files.
Config on the config file takes precedence from the inherited files
Related Tickets & Documents
Checklist before requesting a review
Testing