-
Notifications
You must be signed in to change notification settings - Fork 739
Hide percentage progress until process is closed #4588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nice 👏🏻
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
4e27468 to
dfd7d09
Compare
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Now that Phil's PR is merged, this PR is ready for review.
@pditommaso can you review this one
Not sure what's the difference closed vs terminated and how this information is showed
Closed means the process has created all of the tasks it's going to create (except possibly for retries).
At this point we know how many tasks the process must execute and so we can show a percentage. Before this point, it makes no sense to show a percentage.
Whereas terminated means all tasks completed and the process was terminated.
See my example at the top for how I show this:
- before the process is closed, show a question mark instead of a percentage, but still show the
<completed> of <total> - when a task fails, don't include it in the progress because a task failure doesn't get you any closer to completion, but instead show the number of failed tasks that have been retried vs ignored
1f834a0 to
6454605
Compare
5a93547 to
27345a6
Compare
f6a3696 to
49b58d2
Compare
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Updating this PR to focus on the "process closed" concept.
I think this PR would be a nice improvement, but I don't know anymore if it's worth adding the new trace event. If this onProcessClosed event actually ends up being useful for other things, maybe then we could tack this one on. I'd like to see if it actually ends up being used for garbage collection.
b4b321e to
069653d
Compare
b7b4221 to
c1114bc
Compare
Uh oh!
There was an error while loading. Please reload this page.
Spun off from #4573 (comment) and below
This PR changes the progress logging to not show the percentage until the process is "closed", i.e. all tasks have been received. This prevents Nextflow from overestimating the progress before it knows the total task count of a process. Instead a question mark is shown.
This requires a new trace event
onProcessClosed()in order to know how many tasks a process will execute.As an example, consider a process that executes 4 tasks.
Before all tasks are received (i.e. process is still "open"):
Once all tasks are created:
Once all tasks are completed: