serhiy-bzhezytskyy opened a new issue, #61:
URL: https://github.com/apache/solr-orbit/issues/61

   ### Is your feature request related to a problem?
   
   Follow-up from #60, where @janhoy suggested this as a separate improvement:
   
   > It would of course be elegant if we auto resolved the workload location 
from the stored run. Perhaps as a separate improvement in addition to this more 
explicit one?
   
   Today `aggregate` needs to be told where the workload lives 
(`--workload-repository`, or `--workload-path` after #60), even though it 
already knows *which* workload the runs used. The aggregator reads the workload 
name from the stored test run, then loads the workload spec to walk its 
schedule — so the name comes from the run but the location comes from the 
command line.
   
   ### Describe the solution you'd like
   
   `aggregate` resolves the workload source from the stored test run, so 
neither flag is needed in the common case. The flags stay as an override for 
the case where the workload has since moved.
   
   ### Additional context — what this actually requires
   
   Worth stating up front, because it is more than reading an existing field. 
The stored test run keeps the workload **name**, **params** and **revision**, 
but not where it came from:
   
   - `TestRun.as_dict()` persists `workload`, `workload-params` and 
`workload-revision` (`metrics.py`), and `from_dict()` restores those three.
   - Neither the repository name nor the workload path is persisted, so there 
is currently nothing in `test_run.json` that says whether the run used the 
default repository, a different repository, or a local directory.
   
   So the work is roughly:
   
   1. Persist the workload source in `TestRun.as_dict()` / restore it in 
`from_dict()`.
   2. Have the aggregator prefer the stored source, falling back to the flags.
   3. Handle test runs written before the change, which will not carry the 
field. There is existing precedent for that kind of grace period in 
`metrics.py` (`# TODO: cluster is optional for BWC. This can be removed after 
some grace period.`).
   
   Two things that probably need a decision before implementation:
   
   - **Persisted-format change.** Adding a field to `test_run.json` affects 
files users already have; the fallback behaviour matters more than the happy 
path.
   - **Shared with OpenSearch Benchmark.** `aggregator.py` and the relevant 
part of `metrics.py` are effectively identical to OSB's, so a format change 
here is worth coordinating rather than diverging on. (The three `aggregate` 
fixes were reported in both projects for the same reason — 
opensearch-project/opensearch-benchmark#1095 is the counterpart of #57.)
   
   I can work on it once there is a view on those two points — mainly whether 
the new field should be optional-with-fallback indefinitely or on a deprecation 
clock, and whether it is worth raising with OSB first.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to