Adds an "Indicator" Model and displays it on the toolbar

[zevisert]

Progress for #8608. Adds a new model subclass: Indicator. A list of indicator names can be passed to Figure, just like tools are. This PR just adds a status indicator for the bokeh server websocket, but these could be used for many other applications.

• issues: fixes [Feature request] automatic reconnect to server #8608
• tests added / passed
• release document entry (if new feature or API change)

[bryevdv]

Our codebase standard is not to have leading/trailing space between {}. This will result in diff noise that is otherwise unnecessary. Please adjust here and in other places.

[zevisert]

Changed it, but there's still some noise here regardless. The from keywords were aligned previously, and the line for the new export ServerStatusIdicator uses more columns than where the from columns were before.

Three options:

1. All lines are affected by realigning the from column
2. All lines are affected by removing any alignment
3. Only the additions are modified, but the file has some lines aligned on the from keyword, and some others not aligned

[bryevdv]

@zevisert Thanks for the PR, I've had a similar thought before but not had time to look into it. Just FYI to set expectations, we are prepping for a big release this week, so it may be the weekend/early next we before I can really look into this

[zevisert]

Thanks for the PR,

You're welcome. :)

I've had a similar thought before but not had time to look into it.

Maybe you're referring to #8608? I linked it at the top

Just FYI to set expectations, we are prepping for a big release this week, so it may be the weekend/early next we before I can really look into this

No problem, before you do I'll shed a little more light on the changes here:

• Added a new "Indicator" model to py / js, hopefully can be be useful for any types of indicator icons, think: form validation, plot object selection validation, external API status, bokeh server status (as implemented)
• Indicator models are a little different from normal toolbar buttons, no click handler, and no separate class for plot interactions (ie, no separate views, eg: ButtonToolButtonView / ButtonToolView)
• I seem to be the first to use CustomEvents in js, I want to the status icon to update fully on the client side, not though bokeh document serialization or alike, since if the bokeh server goes down, the icon naturally doesn't update. If you have another preference over CustomEvents let me know.
• [Feature request] automatic reconnect to server #8608 asks for automatic reconnect to server. I re-enabled the _shedule_reconnect function:

  bokeh/bokehjs/src/lib/client/connection.ts

  Lines 99 to 113 in dcc377d

  	protected _schedule_reconnect(milliseconds: number): void {
  	const retry = () => {
  	// TODO commented code below until we fix reconnection to repull
  	// the document when required. Otherwise, we get a lot of
  	// confusing errors that are causing trouble when debugging.
  	if (this.closed_permanently) {
  	logger.info(`Websocket connection ${this._number} disconnected, will not attempt to reconnect`)
  	} else {
  	logger.debug(`Attempting to reconnect websocket ${this._number}`)
  	this.connect()
  	}
  	}
  	setTimeout(retry, milliseconds)
  	}

  
  Bokehjs will now successfully reconnect to the server, but won't continue with it's session probably because the server doesn't have the same state anymore and thus issued a fresh session under a different id. A pull-doc will probably fix this, but be wary. I can remove this again though, as the TODO right next to it warns
• Builds are failing still (obviously), but I think what's breaking them may be outside the scope of this PR. Although, one issue still remains, which is back to CustomEvent. The tests think it's a ReferenceError, but the build passes fine

[bryevdv]

@zevisert I actually had #3393 in mind, and I guess I never fully fleshed out my idea, which was to create new (Bokeh) events to represent busy/unbusy states. But that issue has languished for a long time, and using explicitly enumerable states as property values to drive things also seems reasonable and probably faster to accomplish at this point. What would you think of adding a "busy" or "working" state to this PR? There are definitely pros/cons. Ideally a busy state might "lock" the document (e.g. disabling interactions) until the server unbusies itself. But I am willing to punt on that at least for now. The use case for busy state is when the server instigates some long running code, possibly in a thread but not necessarily (if the author does not mind blocking the user UI)

Regarding re-connection: I would love to work on this with you, but I think there are some additional concerns, so it might be best left to its own PR. E.g even pull-doc may not be sufficient if there are multiple bokeh servers behind a load balancer. So, how to handle that case, etc.

Re: build failing, I actually think it's just some linter errors mostly:

[01:41:09] /home/travis/build/bokeh/bokeh/bokehjs/src/lib/client/connection.ts:6:40
[01:41:09] ERROR: 6:40 semicolon Unnecessary semicolon
[01:41:09] ERROR: 8:1 no-consecutive-blank-lines Consecutive blank lines are forbidden
[01:41:09] ERROR: 25:46 semicolon Unnecessary semicolon
[01:41:09] ERROR: 30:36 trailing-comma Missing trailing comma
[01:41:09] ERROR: 34:3 return-type expected 'status' to have an explicit return type
[01:41:09] ERROR: 35:24 semicolon Unnecessary semicolon
[01:41:09] ERROR: 216:9 prefer-const Identifier 'initStatus' is never reassigned; use 'const' instead of 'let'.
[01:41:09]
[01:41:09] /home/travis/build/bokeh/bokeh/bokehjs/src/lib/client/session.ts:13:3
[01:41:09] ERROR: 13:3 return-type expected 'socket_status' to have an explicit return type
[01:41:09] ERROR: 14:35 semicolon Unnecessary semicolon
[01:41:09]
[01:41:09] /home/travis/build/bokeh/bokeh/bokehjs/src/lib/models/tools/indicators/indicator.ts:2:40
[01:41:09] ERROR: 2:40 semicolon Unnecessary semicolon
[01:41:09] ERROR: 4:31 semicolon Unnecessary semicolon
[01:41:09] ERROR: 10:23 semicolon Unnecessary semicolon
[01:41:09] ERROR: 27:1 no-consecutive-blank-lines Consecutive blank lines are forbidden
[01:41:09]
[01:41:09] /home/travis/build/bokeh/bokeh/bokehjs/src/lib/models/tools/indicators/server_status_indicator.ts:3:38
[01:41:09] ERROR: 3:38 semicolon Unnecessary semicolon
[01:41:09] ERROR: 4:36 semicolon Unnecessary semicolon
[01:41:09] ERROR: 18:90 trailing-comma Missing trailing comma
[01:41:09] ERROR: 19:9 trailing-comma Missing trailing comma
[01:41:09] ERROR: 24:1 no-consecutive-blank-lines Consecutive blank lines are forbidden
[01:41:09] ERROR: 29:32 ban-types Don't use 'String' as a type. Use 'string' instead.
[01:41:09] ERROR: 42:29 semicolon Unnecessary semicolon
[01:41:09] ERROR: 52:64 trailing-comma Missing trailing comma
[01:41:09]
[01:41:09] /home/travis/build/bokeh/bokeh/bokehjs/src/lib/models/tools/toolbar_base.ts:17:64
[01:41:09] ERROR: 17:64 semicolon Unnecessary semicolon

[bryevdv]

For CustomEvent we will need @mattpap to weigh in, I think it the proper way to do it requires targeting ES6 which we are not ready to do I don't think, so a polyfill is maybe necessary

[bryevdv]

I should add, if you have the time and interest to work on this over a slightly longer time frame, I think it might be worth exploring separating the indicator from the status a bit more. I.e. imaging a user wanting to respond to the status changes with CustomJS instead of the default indicator. I guess that really gets back to the notion of document events and callbacks though, so that might be too much to consider at the moment

[bryevdv]

@zevisert I have resolved the merge conclict, FYI.

I'd like to merge this for 1.1.1, but I would also like to punt on the reconnect for now, and attack that in a PR dedicated to that issue. I think there may still be linter errors to fix up as well

[bryevdv]

Hi @zevisert I wanted to check in and see if you still had interest in iterating on this? I think my main concern is simply that plots are basically never likely to have more than one indicator so carving out the extra API and category to add them seems like overkill to me.

[bryevdv]

I really like this idea/direction, but I think it needs a little more discussion. Given the code drift/conflicts and lack of follow-on reponse, I will close this PR now. I'd love to work on this more in the future (at this point, work should probably be against landing-2.0 or against master after 2.0 is released.