Skip to content
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

Render dynamic UI with prodash #13481

Closed
wants to merge 2 commits into from
Closed

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Nov 3, 2021

Switches to rendering the --dynamic-ui with prodash. Although prodash has fewer users than indicatif, the documentation and internals are very solid, and the project maintainer is highly responsive. prodash's conceptual model is also a much closer fit for our workunits: its core data model is a tree, which naturally allows for rendering workunit hierarchy, and dynamically adding and removing tasks.

This change would resolve #13367 by adjusting the number of swimlanes dynamically. But it would also pave a clear path to #11965 to both render task progress via gauges (to show e.g. download progress, or the expected runtime of processes), and potentially by rendering either a child workunit, or the last line of output for long-running tasks as a child of a task.

asciicast

@stuhood
Copy link
Member Author

stuhood commented Nov 3, 2021

This is a draft both because there are a few more issues to nail down (Byron/prodash#9 and Byron/prodash#10), and also because I think that we'll want to see whether we can actually implement some of #11965 before landing this. There are likely also styling changes to be made (light blue is odd, and folks might miss the spinner!).

@thejcannon
Copy link
Member

thejcannon commented Dec 21, 2021

Hey @stuhood what would it take to get this in Pants under some experimental flag?

I'm thinking maybe [GLOBAL].dynamic_ui is now coerced to a string with the following values:

  • false - no dynamic (backwards-compatible)
  • true - alias for spinner (backwards-compatible)
  • spinner - current spinner impl
  • experimental_prodash - this PR

@stuhood
Copy link
Member Author

stuhood commented Jan 6, 2022

Hey @stuhood what would it take to get this in Pants under some experimental flag?

Landing this as experimental would require Byron/prodash#9 landing at a minimum (although for an experimental implementation, I suppose that we could land while using the fork).

The largest question is really: does this seem like an improvement for you? Have you been able to try it out and see what you think of the experience of using it? Or will getting that experience require landing it first?

@thejcannon
Copy link
Member

Last time I used it I remember it being much improved. I can try getting some recordings to compare tomorrow.

@stuhood
Copy link
Member Author

stuhood commented Jan 6, 2022

Last time I used it I remember it being much improved. I can try getting some recordings to compare tomorrow.

Thanks: that would be helpful. I've rebased it.

@thejcannon
Copy link
Member

thejcannon commented Jan 7, 2022

(Sharing videos in Slack)
The difference is stark when < # of available lines processes are executing. Which with 64 cores is often :)

@thejcannon
Copy link
Member

@stuhood closing in favor of #14221

@thejcannon thejcannon closed this Jan 25, 2022
stuhood pushed a commit that referenced this pull request Jan 28, 2022
Adapts #13481 to hide it behind the `--dynamic-ui-renderer=experimental-prodash` flag until it can be stabilized and become the default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic UI should adapt to larger core counts
2 participants