Repository navigation
feat(rpc): Show the progress when building over RPCs. - #16819
Conversation
47002e9 to
bcb8a81
Compare
Signed-off-by: mefyl <quentin@tarides.com>
bcb8a81 to
f8c93b2
Compare
|
The main disadvantage of subscribing to the global count is that it will show unrelated progress on a busy server. For an exaggerated example, consider the main build server currently busy with running some long jobs like some tests. If a client requests a target to be built, it will join the current build, merging their progress. The client may however get a reply that their build is finished before the entire progress finishes. See the I've experimented in the past with having per-requests progress reporting that would give the client a more accurate progress. You can see a recently rebased branch here https://github.com/Alizter/dune/tree/prototype/rpc-request-progress This was just an experiment on my part and I didn't have any near-term plans to finish off this idea. But it may give you some ideas on how you can solve some of the problems that you have encountered. |
Signed-off-by: mefyl <quentin@tarides.com>
rgrinberg
left a comment
There was a problem hiding this comment.
Disregarding the caveats already mentioned, I think we should try to keep the display code as centralized as possible to avoid UI drift in the various build reporting code.
rgrinberg
left a comment
There was a problem hiding this comment.
Going to approve this on agreement on the following:
-
We'd like to consolidate the display code so that we don't have to chase various ends of the code base to update things. This is a thing we'd like to be easy to iterate and improve
-
We accept the limitation that progress is shared.
-
Stick to dense, informative data in the status line. Telling that goals are shared isn't all that helpful or actionable. The number of active goals is sufficient.
Good point. Solving this the 100% correct way by attributing each rule to the target(s) it contributes to seems indeed like a big endeavor, if doable at all without impacting performances too much. May I suggest this compromise, until we assess whether your work can give us the perfect number: if multiple goals are being built, we still display the total progress but indicate that it's shared: I think it achieves most of the feature with almost none of the complexity:
All in all I feel like reporting the progress with the limitation that we may aggregate it with another build is better than not reporting anything. |
Signed-off-by: mefyl <quentin@tarides.com>
|
Following the discussions, this PR evolved into:
The work by @Alizter could make this output better in the future by showing only the progress for the current goal. |
|
To clarify one more thing: the fact that RPC and regular build display should match is not a hard rule. It's just that differences should be clearly motivated as to why is something is only appropriate for one and not the other. Even with this mind, I would still prefer if the entire UX component was consolidated into one place and we handled the differences via conditionals in this component. |
fbcd7b6 to
1c2f4c5
Compare
…goals. Signed-off-by: mefyl <quentin@tarides.com>
Signed-off-by: mefyl <quentin@tarides.com>
1c2f4c5 to
77f17a2
Compare
Signed-off-by: mefyl <quentin@tarides.com>
RPCs round floats to 12 significant digits, so the build start time may be rounded by up to 5ms in the future, which can cause an initial build time of -5ms to be displayed. An imprecision of 5ms is not important for this informative data, so just clamp it to 0 so it does not look weird. Signed-off-by: mefyl <quentin@tarides.com>
Description
When building over RPCs, display progress via the existing dune monitor polling.
Example status line output:
Related Issue and Motivation
When using passive watch mode, building will properly report the end result, but shows no progress. On longer builds taking minutes, this leaves
dune buildstaring at you silently for what is, IMO, an uncomfortable time. I often end up going to the dune watch output terminal/buffer to check progress.There is a slight misfeature were, in a pathological case, we could display the very end of the previous build right before it is canceled. We do send the build request first to mitigate that as much as possible, but it is possible that a progress flashes before we display "restarting current build" and the new progress. I did not find a fix simple enough to justify the change, given how non critical this issue seems to be.
Checklist