Skip to content

Conversation

@kenodegard
Copy link
Contributor

Inspired by the discussion in #1025 (comment).

Instead of using a threaded approach for remove_when_done processing we check for removals as part of the rendering process instead.

@jonathanslenders this feels like a hack at this point, not sure if we'd want the get_progress_bar/set_progress_bar/get_counter/set_counter to be extending the AppSession or whether using this separate ProgressBarSession is preferred.

If we continue down this contexvar path then maybe AppSession should be made more generic to where anything can be set in a session? So we'd introduce something like get_contextvar(key) and set_contextvar(key, value). Just thinking out loud here.

@kenodegard kenodegard force-pushed the progressbar.delayremove3 branch from c4dbf8a to 83ed3e0 Compare January 2, 2020 14:33
@kenodegard kenodegard force-pushed the progressbar.delayremove3 branch from 83ed3e0 to a7c43c3 Compare January 2, 2020 14:35
@kenodegard
Copy link
Contributor Author

Pinging this.

Been using this change for the last week and haven't run into any issues. So I don't have any concerns with implementing remove_when_done as a Conditional.

The only open question is whether the get_progress_bar and get_counter are implemented in the desired way.

@kenodegard kenodegard changed the title [wip] Initial attempt for a Conditional remove_when_done ProgressBar Conditional remove_when_done Apr 13, 2020
@kenodegard
Copy link
Contributor Author

@jonathanslenders let me know if this is of interest or whether I should just close this and implement a monkey patch on my end - thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant