progress messages for jupyterhub 0.9

[clkao]

With jupyterhub/jupyterhub#1767 the pending_messages are rendered:

not super fancy, but informative enough for now.

Closes #141
Closes #148

[clkao]

This is now using the api provided by jupyterhub/jupyterhub#1771

[minrk]

I left a comment about the queue being a class attributed and how to fix that, but I'm not 100% sure that a Queue is what we want. Specifically, progress may be called multiple times, concurrently, so it should behave appropriately when there are multiple consumers of the progress events. As it is now, due to the consumption of events in queue.get, only one consumer will get each event.

Other patterns that could work:

1. share no state between progress calls and create an event reflector for each call (most likely simplest, but most costly if used a lot. It won't be, though)
2. process events into a history, on new progress call, replay history and then connect to queue (queue must support multiple consumers). This is the most complicated, since we need to record the history and also delete it at some point.
3. ignore history, only implement multiple-consumer aspect of 2.

Since this will likely be a common issue, we could try to handle it generically at the JupyterHub level so you don't have to deal with it in each Spawner. What do you think?

[minrk]

This is a class attribute, so all EventReflectors share the same Queue, which I don't think we want.

Use

queue = Instance(Queue, args=(20,)) 

and the queue will be created as an instance attribute.

[minrk]

This will block the main thread while there are no events ready to get. We need an async wait for the queue. Perhaps an asyncio Queue is the right object, but it is not threadsafe, so you will need a little extra work (IOLoop.add_callback) to put messages on the queue.

[clkao]

Thanks for the review @minrk!

It seems reasonable to handle this generically in jupyterhub, as progress can indeed be called multiple times when multiple opened tabs, or if we decide to inspect events in admin ui. this is quite spawner-agnostic.

I suppose a static event array appended by the reflector, with individual queue for generator using an index seems to be the simplest way for doing this? btw that'd be awfully similar to the original implementation before the async progress api ;)

[minrk]

The original implementation couldn't handle multiple watchers or notifying of new events, which are the main issues that we need to solve. To avoid creating multiple reflectors (which is by far the simplest option), we would need to:

1. create a static record of the history of past events
2. register a watcher for each call to receive new events
3. yield events, transitioning carefully from the static list to listeners for new events
4. properly destroy the history when done, to avoid filling up local memory

I'll have a stab at doing this on the jupyterhub side. If Spawner progress yielded events from the beginning every time, this would be vastly simpler, which might be my preference.

[minrk]

@clkao I opened #169 because Watch creating a new Client for every spawn is still going to be a problem without nipping the extra threads in the bud. Hopefully after that, we should be safe.

[minrk]

This while loop needs to end at some point. How do you signal that there will be no more events?

[clkao]

@minrk thanks for the review. I added a check to break if the reflector is stopped for the generator.

However it is unclear to me what happens when the event stream connection is closed. Does the async with aclosing in the api handler finalize the call to progress generator?

[minrk]

Does the async with aclosing in the api handler finalize the call to progress generator?

Yes, I believe it does.

[minrk]

Let's give this a try. Thanks!