-
Notifications
You must be signed in to change notification settings - Fork 111
Olog websockets #3521
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
base: master
Are you sure you want to change the base?
Olog websockets #3521
Conversation
this is great, |
@shroffk, do you mean a new release of Olog or Phoebus? Or both? |
…and show message (not in dialog)
(msg, ex) -> { | ||
searchInProgress.set(false); | ||
ExceptionDetailsErrorDialog.openError(Messages.LogbooksSearchFailTitle, ex.getMessage(), null); | ||
//ExceptionDetailsErrorDialog.openError(Messages.LogbooksSearchFailTitle, ex.getMessage(), null); |
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.
Commented-out code.
public void shutdown() { | ||
cancelPeriodSearch(); | ||
@Override | ||
public void handleWebSocketMessage(String message) { |
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.
If two new log entries are created more or less simultaneously, will this function be called in parallel, or will the two calls occur sequentially (in that case, presumably on the same thread)?
public void handleWebSocketMessage(String message) { | ||
try { | ||
WebSocketMessage webSocketMessage = objectMapper.readValue(message, WebSocketMessage.class); | ||
if (webSocketMessage.messageType().equals(MessageType.NEW_LOG_ENTRY)) { |
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.
Should the two other message types (MessageType.LOG_ENTRY_UPDATED
and MessageType.SHOW_BANNER
) also be handled by this function?
|
||
setupTextAreaContextMenu(); | ||
|
||
//doStompStuff(); |
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.
Commented-out code.
} | ||
} | ||
|
||
private void doStompStuff(){ |
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.
This function is not called. (The only call to it (on line 559
) is commented out.)
} | ||
|
||
/** | ||
* |
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.
Empty comment.
* Disconnects the STOMP session if non-null and connected. | ||
*/ | ||
public void disconnect() { | ||
if(stompSession != null && stompSession.isConnected()) { |
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.
Does this class need to be made thread-safe, perhaps by means of synchronization? E.g., could stompSession
be assigned to between the check stompSession != null
, the check stompSession.isConnected()
, and the call stompSession.disconnect()
?
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.
I do not think this needs to be synchronized. stompSession
is null
only initially, i.e. as long as API client has not called connect
and the connection attempt succeeds. Hence the null check.
Additions based on feedback from collaboration peers: add backwards compatibility, i.e. if service does not support web sockets, fall back to periodic polling of log entries. |
This PR adds web sockets to Olog ES clients (Log Entry Table and Log Entry Calendar).
As the implementation uses Spring Framework web socket libs, new dependencies are added, total ~2MB. If Olog is not used, site specific products should exclude
core-websocket
to avoid these dependencies.A dedicated class
WebSocketClientService
is intended to be somewhat generic. It handles reconnections as well as keep-alive (aka ping/pong).The idea is to skip polling for new log entries twice per minute, but rather rely on web socket messages dispatched from the associated Olog service.
NOTE: the Olog web client will through this PR depend on Olog/phoebus-olog#220.