core(runner): abstract gather phase out of runner #11623
Conversation
brendankenny
left a comment
brendankenny
left a comment
There was a problem hiding this comment.
One thing we've talked about at various times in the past has been splitting out gathering and auditing into even more distinct steps, basically turning gather-runner into the -G runner and runner into the -A runner (an audit-runner, if you will :).
That's another option here, basically passing in artifacts (procured however) into runner, which would avoid the inversion of control of the callback. I have a super ancient branch that explored this a bit, adding an explicit audit-runner file while runner remains as the orchestrator of the two, but I don't think there's any reason to necessarily go that way with runner...instead code calling gather() and then audit() could be kicked back up to index.
That does involve more code and test rearranging, though, than this very succinct change.
|
I agree with that overall direction and I think we'll get there when all is said and done at the end of this (the changes for timespan will force that issue a bit more than the phase 1 snapshot mode does where some key concepts violate the existing runner structure). I think the main part of this decision comes down to how would others on the team prefer that FR affect other areas? Would early "unnecessary" refactors without understanding the need for them yet be preferable to finding out too late that some big refactor might be necessary later? This PR reflects my preference for deferral in case things change, but if, as a whole, we'd rather frontload some of those changes before FR functionality arrives that also seems OK.
My goal was to avoid that for the purposes of early FR unblocking :) |
Well, if you're open to the general design and willing to review, one option is to unblock FR with this PR, then I could make the change which would allow frontloading without slowing down the other work you have in the pipeline (assuming everything is compatible, which is seems like it would be). |
|
Sounds good 👍 |
4 participants
Summary
The runner related changes for FR phase 1. See #11622 for how it will eventually come together. Goal here is to eliminate the reliance of the Runner on an explicit connection and abstract away the gathering.
Related Issues/PRs
ref #11313