Skip to content
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

requestCommInfo not honoring target name #6947

Closed
kb0304 opened this issue Aug 5, 2019 · 8 comments · Fixed by #6949
Closed

requestCommInfo not honoring target name #6947

kb0304 opened this issue Aug 5, 2019 · 8 comments · Fixed by #6949

Comments

@kb0304
Copy link

@kb0304 kb0304 commented Aug 5, 2019

Describe the bug
When developing an extension which sends custom comm messages, and with ipywidgets' jupyterlab-manager installed, the messages even with target other than jupyter.widget are being intercepted by jupyterlab-manager

Why the requestCommInfo is not honoring target name here?

Expected behavior
Messages only with target jupyter.widget are intercepted

  • Browser [chrome ]
  • JupyterLab [1.0.2]
  • @jupyter-widgets/jupyterlab-manager@1.0.1
@kb0304
Copy link
Author

@kb0304 kb0304 commented Aug 5, 2019

Turns out they're using target here instead of target_name
const reply = await this._context.session.kernel.requestCommInfo({target: this.comm_target_name});

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 5, 2019

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 5, 2019

Thanks for finding and reporting this, and finding the underlying cause. Let's move the discussion over to the ipywidgets issue, since that's where the problem is.

@jasongrout jasongrout closed this Aug 5, 2019
@jasongrout jasongrout added this to the Reference milestone Aug 5, 2019
@Madhu94
Copy link
Collaborator

@Madhu94 Madhu94 commented Aug 5, 2019

@jasongrout I believe the KernelMessage type should be fixed in lab first - here, before you could fix ipywidgets (TS errors may surface)

@Madhu94
Copy link
Collaborator

@Madhu94 Madhu94 commented Aug 5, 2019

If we change the type, would that constitute a breaking change for the services package? If so, could renaming target to target_name here may be a quicker temporary fix?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 5, 2019

@jasongrout I believe the KernelMessage type should be fixed in lab first - here, before you could fix ipywidgets (TS errors may surface)

Good catch. This is deeper than I thought.

If we change the type, would that constitute a breaking change for the services package?

Technically, yes, but it's also a pretty clear bug-fix, in that it is not following the spec.

@jasongrout jasongrout reopened this Aug 5, 2019
jasongrout added a commit to jasongrout/jupyterlab that referenced this issue Aug 5, 2019
…tible way.

This workaround should be removed in services 5.0.

Fixes jupyterlab#6947
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 5, 2019

What do you think of #6949 ?

@Madhu94
Copy link
Collaborator

@Madhu94 Madhu94 commented Aug 5, 2019

@lock lock bot locked as resolved and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants