Let aggregate load a workload from --workload-path

[serhiy-bzhezytskyy]

Description

add_workload_source() offers a workload either as a repository or as a local path, and run, list and info all use it. aggregate defined only its own --workload-repository, so a test run made against a workload on disk could not be aggregated afterwards — the aggregator loads the workload to read its schedule, and a repository lookup fails for a workload that only exists as a path:

$ opensearch-benchmark aggregate --test-runs=<id1>,<id2> --workload-path=/path/to/workload
opensearch-benchmark: error: unrecognized arguments: --workload-path=/path/to/workload

The aggregate parser now uses the same add_workload_source helper as run, so the two options stay mutually exclusive and --workload-revision comes along with them. Dispatch calls configure_workload_params with command_requires_workload=False, matching how list already handles a command that takes a workload source but no --workload — aggregate gets the workload name from the stored test run.

One behavioural detail: a path sets workload.path, which is what makes the loader choose SimpleWorkloadRepository. So when a path was given, aggregate() no longer names a repository — doing both would send the loader looking for the workload inside that repository instead. That guard is load-bearing, since argparse's default for --workload-repository is the string "default" rather than None.

Issues Resolved

Resolves #1095

Testing

• New functionality includes testing

• Two new tests: test_aggregate_names_the_workload_repository (unchanged behaviour when a repository is used) and test_aggregate_leaves_a_workload_path_alone (no repository.name is configured when a path is given).

• The shared mock_args fixture previously supplied no workload_path, so the aggregator saw a Mock attribute that happens to be truthy. It is now explicit — that fixture change is what makes the first test meaningful rather than accidentally passing.

• CLI verified by hand: aggregate --workload-path=<dir> is accepted where it previously errored, aggregation succeeds end to end, and combining it with --workload-revision is correctly rejected.

• Full suite: 1424 passed, 5 skipped (baseline on main is 1422). pylint clean.

Notes

Found while using Apache solr-orbit, a Python port of OSB with the same argparse structure, to run a multi-configuration benchmark campaign. The same change is proposed there as apache/solr-orbit#60. Third of three aggregate gaps I hit, and the only one that is a parity gap rather than a bug.

There is a case for going further — a test run already records the workload it used, so aggregate could resolve the spec from the stored run and need no workload-source argument at all. I've gone with the parity fix because it is the smaller change and matches the existing pattern, but I'm open to the other shape if you'd rather have it.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 6ee7b81)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 6ee7b81
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid overriding workload name when path given When --workload-path is provided, the workload name derived from the test run should not overwrite the path-based workload configuration set by configure_workload_params. Setting workload.name unconditionally may cause the loader to look up a workload by name instead of using the provided path. Guard the workload.name assignment with the same workload_path check. osbenchmark/aggregator.py [297-299] if not self.args.workload_path: self.config.add(config.Scope.applicationOverride, "workload", "repository.name", self.args.workload_repository) -self.config.add(config.Scope.applicationOverride, "workload", "workload.name", self.test_run.workload) + self.config.add(config.Scope.applicationOverride, "workload", "workload.name", self.test_run.workload) Suggestion importance[1-10]: 6 __ Why: The suggestion raises a plausible concern that setting workload.name unconditionally may conflict with a workload path already configured by configure_workload_params. However, without confirmation of how load_workload prioritizes workload.path vs workload.name, the fix may not be strictly necessary; still, the logic parallel to the repository guard is reasonable.	Low

---

Previous suggestions

Suggestions up to commit 4aea84c

Category	Suggestion	Impact
General	Avoid overriding workload name for path workloads When --workload-path is provided, workload.name should not be forced from test_run.workload either, since configure_workload_params already sets workload.path and the workload name for a path-based workload. Overwriting workload.name may cause the loader to misinterpret the source. Guard this call the same way as the repository call. osbenchmark/aggregator.py [277-279] if not self.args.workload_path: self.config.add(config.Scope.applicationOverride, "workload", "repository.name", self.args.workload_repository) -self.config.add(config.Scope.applicationOverride, "workload", "workload.name", self.test_run.workload) + self.config.add(config.Scope.applicationOverride, "workload", "workload.name", self.test_run.workload) Suggestion importance[1-10]: 5 __ Why: The suggestion raises a legitimate concern: when workload_path is set, configure_workload_params likely already handles workload identification, so overwriting workload.name could confuse the loader. However, without deeper knowledge of how load_workload handles path-based workloads, the impact is uncertain and could also break existing behavior.	Low

[OVI3D0]

LGTM but there are conflicts from the other PR(s) so we'll just need to do another quick rebase before i merge. Thank you @serhiy-bzhezytskyy !

[github-actions]

Persistent review updated to latest commit e8f1474

[OVI3D0]

@serhiy-bzhezytskyy this will need one more rebase because of #1096 :)

[serhiy-bzhezytskyy]

Rebased onto main with #1096 in.

[github-actions]

Persistent review updated to latest commit 6ee7b81