widget_class information on comm_open should be placed in data

[lbustelo]

The current code merged in #461 places the information about widget_class in a the meta-data section of the comm_open message. This is a huge deal, because nowhere in the spec around Comm support (and I'm basing the statement on this) does it say that handlers to comm message (open, msg, close, info) need access to the meta-data area. That being the case, both the Comm support in IRKernel and Toree/SparkKernel, do not make this area of the message available to handlers. In my opinion, the goal of Comm is to simplify communication and not to expose all the pieces of the Kernel Protocol.

I propose that this is changed so that the information about widget_class is passed within the data area, a much easier fix for declarativewidgets and do not involve changes to the kernels.

/cc @SylvainCorlay @jdfreder @jasongrout @parente

[lbustelo]

Some relevant pieces of code

1. The R support of comm_open https://github.com/IRkernel/IRkernel/blob/master/R/comm_manager.r#L118
2. The Toree support for comm_open https://github.com/apache/incubator-toree/blob/2aa8ce12bbf0aa08dfe4e52731aa5fb1a9d520eb/protocol/src/main/scala/org/apache/toree/kernel/protocol/v5/content/CommOpen.scala#L23

[SylvainCorlay]

@lbustelo ok, do you think we could have the widget_class info in both data and metadata for now so that things work in the R and scala kernels?

[lbustelo]

I think that is fine. I still think that 'data' is the better place. If not, then what is the 'data' piece of the 'comm_open' for?

Gino B.

On Apr 19, 2016, at 8:56 PM, Sylvain Corlay [email protected] wrote:

@lbustelo ok, do you think we could have the widget_class info in both data and metadata for now so that things work in the R and scala kernels?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[SylvainCorlay]

Eventually it should probably used for passing state. (Like from the backend to the frontend.)

[lbustelo]

I see... Not sure placing the widget class in data would preclude you from putting state along with it in the 'comm_open'. As it stands, the data area is really an open ended map specific to the target_name. In this case ipywidgets gets to define what those payloads look like.

[SylvainCorlay]

In any case, we will need to support both for some time (whether we choose data or metadata eventually). We should coordinate on this with @jdfreder and @jasongrout (because of the shims used for jupyterlab).

[jasongrout]

Conceptually, +1 for making the widget_class part of data instead of metadata, since the widget_class is targeted at the widget manager, rather than the comm layer, i.e., widget_class is data that the widget manager needs to handle the comm_open message.

[SylvainCorlay]

We should probably make it symmetrical with the backend-to frontend comm_open...

[jasongrout]

@SylvainCorlay - yes, I think it should be symmetrical.

[lbustelo]

@SylvainCorlay will this be part of a 5.0.1?