mit-teaching-systems-lab / dcss-remote-ai-integration

1 stars 2 forks source link

Socket connections #2

Open rcmurray opened 3 years ago

rcmurray commented 3 years ago

@rwaldron, I believe we want the capability to send messages to specific users without broadcasting to everyone. I think that means we need one socket per user.

rcmurray commented 3 years ago

@rwaldron, I would like the capability to include a path in the socket definition. I got communication with your index.html file going on Wednesday by adding a path parameter as follows:

  const socket = io(endpoint, {
    transports,
    path: "/bazsocket/",
    auth: {
      token
    }
rcmurray commented 3 years ago

@rwaldron, as I'm coding this up, I realize that I don't think Bazaar ever uses a request-response protocol. Following are some reasons.

A. To facilitate collaboration, Bazaar intentionally steps back to see if other users (typically human) will respond before it decides whether to step in (so Bazaar may not respond). Bazaar responding to every message (in this case, "request") would tend to take over the conversation and thus inhibit effective collaboration. B. Even when Bazaar has a specific response to a specific message, it may have other thing(s) to say as well, with a result that the response may come out of order. E.g.: -- request -- interjection -- response C. There may be requests from multiple users at once -- perhaps duplicate -- but Bazaar usually responds to the group as a whole. D. A response may no longer be appropriate if the collaboration has moved on.

For these reasons, I think it may work best to use uncoupled "requests" and "interjections." For Bazaar, these are both currently just "updatechat," which would be easiest for me to implement for this application, but I understand that you're planning for multiple possible endpoints and so I think we can "get with the program" of requests and interjections.

What do you think?

rcmurray commented 3 years ago

@rwaldron, since an interjection may not be in response to a request, we can't guarantee that the interjection will contain a 'context' value that is the same as "the request." I hope that a more general value that identifies a room context and perhaps a user will be sufficient.

rcmurray commented 3 years ago

@rwaldron, this connection is requiring some special handling by the Bazaar server -- e.g., to start an agent as soon as we receive a socket connection request -- and so it would be good (i.e., avoid hackiness) if we could receive some sort of unique yet constant identifier for the DCS endpoint along with the other information we need for a new connection:

  1. DCSS identifier constant
  2. agent -- e.g. 'politeness'
  3. roomName

And if the initial connection includes a user joining the chat:

  1. userID -- userIDs must be unique since userNames can be duplicated
  2. userName

We may be able to associate the DCSS identifier with the socket so that we will know for subsequent messages that this is communication with DCSS.

rwaldron commented 3 years ago

I believe we want the capability to send messages to specific users without broadcasting to everyone.

Yes, that's what the token was originally

I think that means we need one socket per user.

This is the wrong model. DCSS is the Client, which will open one socket per scenario execution (which corresponds to a chat, or room), to each remote service. The remote service will receive information that makes clear who the user is for each message it receives. It's up to the server to manage coordination on it's own. This is what the Socket.io "room" feature enables.

Edit: After further consideration, I think I'm sufficiently convinced of the socket-per-user model. I have some work to do to clean up the examples and the docs.

rwaldron commented 3 years ago

I've just made some updates that reflect our discussion on Thursday and my notes here has well. I have more work to do on annotations

rwaldron commented 3 years ago

I've eliminated the context object. All socket connections are now "per user".

rwaldron commented 3 years ago

I would like the capability to include a path in the socket definition. I got communication with your index.html file going on Wednesday by adding a path parameter as follows:

That kind of configuration will be done through the admin console in the teacher moments application. When providing docs to whomever manages those entities, they will need to know that path: /bazsocket/.