Skip to content

Conversation

@jasongrout
Copy link
Member

A problem can happen when two messages come in for different
comms, where the second depends on the first (for example, the
first might be a message setting the state of a widget, and the
second triggering a view creation for the widget). Since comm
message queues are independent of each other, the second message
could be executed before the first message.

@jasongrout
Copy link
Member Author

We probably only need to merge either this or #7838, but not both. #7838 is a stronger approach, while this approach allows for some asynchronicity in processing different types of kernel messages.

@jasongrout
Copy link
Member Author

CCing @jdfreder, @minrk for comments

@jdfreder jdfreder added this to the 3.1 milestone Feb 24, 2015
@jdfreder
Copy link
Contributor

My initial impression is that I prefer this over #7838 for two reasons:

  • This only affects comms and all code built on top of them which means it potentially has much less side effects than the other PR, which digs as deep as the websockets.
  • This is much less code.

@minrk
Copy link
Member

minrk commented Feb 24, 2015

I'm a bit torn. I probably prefer this for 3.x due to its relative simplicity, but in the long run, I think the synchronization should probably be done at the lower level, forcing ordering at the channel or socket level, rather than per message type.

Now that we understand working with promises and 🐴 better, I think for 4.0 we should take a step back and rework the callbacks/event handling of messages to be consistent across the board, rather than the inconsistent mixture of approaches we have right now. Almost all of the problems we have with respect to these things are specific to widgets, and we have to be realistic about what widgets are going to be in 3.x.

@minrk
Copy link
Member

minrk commented Feb 24, 2015

counterpoint: #7838 includes #7780, which I do think we should have in 3.1, so maybe #7838 is the way to go. I'm just not sure yet.

@jasongrout
Copy link
Member Author

I like #7838 better. It allows any message handler to be synchronous, not just comms. It's simpler conceptually.

A problem can happen when two messages come in for different
comms, where the second depends on the first (for example, the
first might be a message setting the state of a widget, and the
second triggering a view creation for the widget).  Since comm
message queues are independent of each other, the second message
could be executed before the first message.

Thanks to @dmadeka for reporting an error that led to discovering this issue.
@minrk
Copy link
Member

minrk commented Mar 3, 2015

closing in favor of #7838

@minrk minrk closed this Mar 3, 2015
@minrk minrk modified the milestones: no action, 3.1 Mar 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants