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

Add "widget_id" filter to "request_states" message, fixing + refactoring some CI #3833

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dby-tmwctw
Copy link

Explanation of motivation

There is a bug in iPyWidgets state handling that happened quite frequently: If the comm target is already closed (when the widget is closed/deleted) before the frontend sends the request_state, Jupyter kernel swallows the request_state and does nothing (as the comm target does not exist), leading to the frontend thinking backend hangs. As a result, we need a source of truth for widget state when a widget is closed, and Widget Manager is currently the best source of truth since it have all the widget states.

In order to solve the bug, the frontend can now ask Widget for the state with request_states. We do not want to send back the entire chunk of all widget states when we only need one state, we added the filter for request_states. This will send back the state for a specific widget frontend is requesting, and if the response coming back is an empty dictionary frontend knows the widget doesn't exist any more. This solves the bug I mentioned above.

Testing fixes

The testing pipeline's DummyComm have default comm_id, but widgets' model_id is set to be its self.comm.comm_id. This means all widgets created in test environment have same model_id. Due to the way we set-up _instances (_instances[self.model_id] = self), that effectively means we only have 1 active widget available. This is fine in most cases, but in our case we do need different widgets co-exist for filtering verification. Hence, not the comm_id is randomly generated.

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch dby-tmwctw/ipywidgets/filter-widget-boyuan

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.

1 participant