diff --git a/packages/base-manager/test/src/manager_test.ts b/packages/base-manager/test/src/manager_test.ts index 47032e9d3a..2acb8a70d7 100644 --- a/packages/base-manager/test/src/manager_test.ts +++ b/packages/base-manager/test/src/manager_test.ts @@ -180,7 +180,7 @@ describe('ManagerBase', function () { }, }, metadata: { - version: '3.0.0', + version: '2.1.0', }, }); expect(model.comm).to.equal(comm); @@ -243,7 +243,7 @@ describe('ManagerBase', function () { }, buffers: [new DataView(new Uint8Array([1, 2, 3]).buffer)], metadata: { - version: '3.0.0', + version: '2.1.0', }, }); expect(model.comm).to.equal(comm); diff --git a/packages/base/package.json b/packages/base/package.json index 117e9d2d26..2af1c599a9 100644 --- a/packages/base/package.json +++ b/packages/base/package.json @@ -26,6 +26,7 @@ "test:coverage": "npm run build:test && webpack --config test/webpack-cov.conf.js && karma start test/karma-cov.conf.js", "test:unit": "npm run test:unit:firefox && npm run test:unit:chrome", "test:unit:chrome": "npm run test:unit:default -- --browsers=Chrome", + "test:unit:chrome:debug": "npm run test:unit:default -- --browsers=Chrome --single-run=false", "test:unit:default": "npm run build:test && karma start test/karma.conf.js --log-level debug", "test:unit:firefox": "npm run test:unit:default -- --browsers=Firefox", "test:unit:firefox:headless": "npm run test:unit:default -- --browsers=FirefoxHeadless", diff --git a/packages/base/src/version.ts b/packages/base/src/version.ts index 5249af2cd2..3897463d6f 100644 --- a/packages/base/src/version.ts +++ b/packages/base/src/version.ts @@ -3,4 +3,4 @@ export const JUPYTER_WIDGETS_VERSION = '2.0.0'; -export const PROTOCOL_VERSION = '3.0.0'; +export const PROTOCOL_VERSION = '2.1.0'; diff --git a/packages/base/src/widget.ts b/packages/base/src/widget.ts index dfa740e7a4..54ff15e17a 100644 --- a/packages/base/src/widget.ts +++ b/packages/base/src/widget.ts @@ -115,7 +115,7 @@ export class WidgetModel extends Backbone.Model { attributes: Backbone.ObjectHash, options: IBackboneModelOptions ): void { - this.expectedEchoMsgIds = {}; + this.expectedEchoMsgIds = new Map(); this.attrsToUpdate = new Set(); super.initialize(attributes, options); @@ -224,32 +224,32 @@ export class WidgetModel extends Backbone.Model { const method = data.method; switch (method) { case 'update': + case 'echo_update': this.state_change = this.state_change .then(() => { const state = data.state; - const buffer_paths = data.buffer_paths || []; - const buffers = msg.buffers || []; + const buffer_paths = data.buffer_paths ?? []; + const buffers = msg.buffers?.slice(0, buffer_paths.length) ?? []; utils.put_buffers(state, buffer_paths, buffers); - if (msg.parent_header && data.echo) { + + if (msg.parent_header && method === 'echo_update') { const msgId = (msg.parent_header as any).msg_id; // we may have echos coming from other clients, we only care about // dropping echos for which we expected a reply - const expectedEcho = data.echo.filter((attrName: string) => - Object.keys(this.expectedEchoMsgIds).includes(attrName) + const expectedEcho = Object.keys(state).filter((attrName) => + this.expectedEchoMsgIds.has(attrName) ); expectedEcho.forEach((attrName: string) => { - // we don't care about the old messages, only the one send with the - // last msgId + // Skip echo messages until we get the reply we are expecting. const isOldMessage = - this.expectedEchoMsgIds[attrName] !== msgId; + this.expectedEchoMsgIds.get(attrName) !== msgId; if (isOldMessage) { - // get rid of old updates + // Ignore an echo update that comes before our echo. delete state[attrName]; } else { - // we got our confirmation, from now on we accept everything - delete this.expectedEchoMsgIds[attrName]; - // except, we plan to send out a new state for this soon, so we will - // also ignore the update for this property + // we got our echo confirmation, so stop looking for it + this.expectedEchoMsgIds.delete(attrName); + // Start accepting echo updates unless we plan to send out a new state soon if ( this._msg_buffer !== null && Object.prototype.hasOwnProperty.call( @@ -263,6 +263,7 @@ export class WidgetModel extends Backbone.Model { }); } return (this.constructor as typeof WidgetModel)._deserialize_state( + // Combine the state updates, with preference for kernel updates state, this.widget_manager ); @@ -498,8 +499,8 @@ export class WidgetModel extends Backbone.Model { } } rememberLastUpdateFor(msgId: string) { - [...this.attrsToUpdate].forEach((attrName) => { - this.expectedEchoMsgIds[attrName] = msgId; + this.attrsToUpdate.forEach((attrName) => { + this.expectedEchoMsgIds.set(attrName, msgId); }); this.attrsToUpdate = new Set(); } @@ -679,7 +680,7 @@ export class WidgetModel extends Backbone.Model { // keep track of the msg id for each attr for updates we send out so // that we can ignore old messages that we send in order to avoid // 'drunken' sliders going back and forward - private expectedEchoMsgIds: any; + private expectedEchoMsgIds: Map; // because we don't know the attrs in _handle_status, we keep track of what we will send private attrsToUpdate: Set; } diff --git a/packages/schema/messages.md b/packages/schema/messages.md index a7b6f4cb4d..bbcbfe7599 100644 --- a/packages/schema/messages.md +++ b/packages/schema/messages.md @@ -292,27 +292,31 @@ The `data.state` and `data.buffer_paths` values are the same as in the `comm_ope See the [Model state](jupyterwidgetmodels.latest.md) documentation for the attributes of core Jupyter widgets. -#### Synchronizing multiple frontends: `update` with echo +#### Synchronizing multiple frontends: `echo_update` -Starting with protocol version `3.0.0` the kernel can send a special update message back, to allow all connected frontends to be in sync with the kernel state. This allows multiple frontends to be connected to a single kernel but also resolves a possible out of sync situation when the kernel and a frontend send out an update message at the same time, causing both to think they have the latest state. - -In protocol version `3.0.0` the kernel is considered the single source of truth and is expected to send back to the frontends an update message that contains an extra list of keys to indicate which keys in the update are send back to the frontends as a reaction to an update received from a frontend. +Starting with protocol version `2.1.0`, `echo_update` messages from the kernel to the frontend are optional update messages for echoing state in messages from a frontend to the kernel back out to all the frontends. ``` { 'comm_id' : 'u-u-i-d', 'data' : { - 'method': 'update', + 'method': 'echo_update', 'state': { }, 'buffer_paths': [ ] - 'echo': [ ] } } ``` -In situations where a user does many changes to a widget on the frontend (e.g. moving a slider), the frontend will receive from the kernel many update messages (with the echo key set) from the kernel that can be considered old values. A frontend can choose to ignore all updates that are not originating from the last update it send to the kernel. This can be implemented by keeping track of the `msg_id` for each attribyte for which we send out an update message to the kernel, and ignoring all updates as a result from an `echo` for which the [`msg_id` of the parent header](https://jupyter-client.readthedocs.io/en/latest/messaging.html#parent-header) is not equal to `msg_id` we kept track of. +The Jupyter comm protocol is asymmetric in how messages flow: messages flow from a single frontend to a single kernel, but messages are broadcast from the kernel to *all* frontends. In the widget protocol, if a frontend updates the value of a widget, the frontend does not have a way to directly notify other frontends about the state update. The `echo_update` optional messages enable a kernel to broadcast out frontend updates to all frontends. This can also help resolve the race condition where the kernel and a frontend simultaneously send updates to each other since the frontend now knows the order of kernel updates. + +The `echo_update` messages enable a frontend to optimistically update its widget views to reflect its own changes that it knows the kernel will yet process. These messages are intended to be used as follows: +1. A frontend model attribute is updated, and the frontend views are optimistically updated to reflect the attribute. +2. The frontend queues an update message to the kernel and records the message id for the attribute. +3. The frontend ignores updates to the attribute from the kernel contained in `echo_update` messages until it gets an `echo_update` message corresponding to its own update of the attribute (i.e., the [parent_header](https://jupyter-client.readthedocs.io/en/latest/messaging.html#parent-header) id matches the stored message id for the attribute). It also ignores `echo_update` updates if it has a pending attribute update to send to the kernel. Once the frontend receives its own `echo_update` and does not have any more pending attribute updates to send to the kernel, it starts applying attribute updates from `echo_update` messages. + +Since the `echo_update` update messages are optional, and not all attribute updates may be echoed, it is important that only `echo_update` updates are ignored in the last step above, and `update` message updates are always applied. -For situations where sending back an echo update for a property is considered too expensive, we have implemented an opt-out mechanism in ipywidgets. A trait can have a `no_echo` metadata attribute to flag that the kernel should not send back an update to the frontends. We suggest other implementations implement a similar opt-out mechanism. +Implementation note: For attributes where sending back an `echo_update` is considered too expensive or unnecessary, we have implemented an opt-out mechanism in the ipywidgets package. A model trait can have the `echo_update` metadata attribute set to `False` to flag that the kernel should never send an `echo_update` update for that attribute to the frontends. Additionally, we have a system-wide flag to disable echoing for all attributes via the environment variable `JUPYTER_WIDGETS_ECHO`. For ipywdgets 7.7, we default `JUPYTER_WIDGETS_ECHO` to off (disabling all echo messages) and in ipywidgets 8.0 we default `JUPYTER_WIDGETS_ECHO` to on (enabling echo messages). #### State requests: `request_state` diff --git a/python/ipywidgets/ipywidgets/_version.py b/python/ipywidgets/ipywidgets/_version.py index a16e0486e4..4c0c1ca148 100644 --- a/python/ipywidgets/ipywidgets/_version.py +++ b/python/ipywidgets/ipywidgets/_version.py @@ -3,7 +3,7 @@ __version__ = '8.0.0b1' -__protocol_version__ = '3.0.0' +__protocol_version__ = '2.1.0' __control_protocol_version__ = '1.0.0' # These are *protocol* versions for each package, *not* npm versions. To check, look at each package's src/version.ts file for the protocol version the package implements. diff --git a/python/ipywidgets/ipywidgets/widgets/tests/test_set_state.py b/python/ipywidgets/ipywidgets/widgets/tests/test_set_state.py index eddb4c779c..0564dc5b94 100644 --- a/python/ipywidgets/ipywidgets/widgets/tests/test_set_state.py +++ b/python/ipywidgets/ipywidgets/widgets/tests/test_set_state.py @@ -90,12 +90,18 @@ def test_set_state_transformer(): )) # Since the deserialize step changes the state, this should send an update assert w.comm.messages == [((), dict( + buffers=[], + data=dict( + buffer_paths=[], + method='echo_update', + state=dict(d=[True, False, True]), + ))), + ((), dict( buffers=[], data=dict( buffer_paths=[], method='update', state=dict(d=[False, True, False]), - echo=['d'], )))] @@ -117,16 +123,15 @@ def test_set_state_data_truncate(): d={'data': data}, )) # Get message for checking - assert len(w.comm.messages) == 1 # ensure we didn't get more than expected - msg = w.comm.messages[0] + assert len(w.comm.messages) == 2 # ensure we didn't get more than expected + msg = w.comm.messages[1] # Assert that the data update (truncation) sends an update buffers = msg[1].pop('buffers') assert msg == ((), dict( data=dict( method='update', - state=dict(d={}, a=True), - buffer_paths=[['d', 'data']], - echo=['a', 'd'], + state=dict(d={}), + buffer_paths=[['d', 'data']] ))) # Sanity: @@ -181,8 +186,8 @@ def test_set_state_cint_to_float(): ci = 5.6 )) # Ensure an update message gets produced - assert len(w.comm.messages) == 1 - msg = w.comm.messages[0] + assert len(w.comm.messages) == 2 + msg = w.comm.messages[1] data = msg[1]['data'] assert data['method'] == 'update' assert data['state'] == {'ci': 5} @@ -265,11 +270,13 @@ def _propagate_value(self, change): assert widget.value == 2 assert widget.other == 11 - # we expect only single state to be sent, i.e. the {'value': 42.0} state - msg = {'method': 'update', 'state': {'value': 2.0, 'other': 11.0}, 'buffer_paths': [], 'echo': ['value']} + msg = {'method': 'echo_update', 'state': {'value': 42.0}, 'buffer_paths': []} call42 = mock.call(msg, buffers=[]) - calls = [call42] + msg = {'method': 'update', 'state': {'value': 2.0, 'other': 11.0}, 'buffer_paths': []} + call2 = mock.call(msg, buffers=[]) + + calls = [call42, call2] widget._send.assert_has_calls(calls) @@ -288,7 +295,7 @@ class ValueWidget(Widget): assert widget.value == 42 # we expect this to be echoed - msg = {'method': 'update', 'state': {'value': 42.0}, 'buffer_paths': [], 'echo': ['value']} + msg = {'method': 'echo_update', 'state': {'value': 42.0}, 'buffer_paths': []} call42 = mock.call(msg, buffers=[]) calls = [call42] @@ -324,17 +331,21 @@ def _square(self, change): # we expect this to be echoed # note that only value is echoed, not square - msg = {'method': 'update', 'state': {'square': 64, 'value': 8.0}, 'buffer_paths': [], 'echo': ['value']} + msg = {'method': 'echo_update', 'state': {'value': 8.0}, 'buffer_paths': []} call = mock.call(msg, buffers=[]) + + msg = {'method': 'update', 'state': {'square': 64}, 'buffer_paths': []} + call2 = mock.call(msg, buffers=[]) + - calls = [call] + calls = [call, call2] widget._send.assert_has_calls(calls) def test_no_echo(): - # in cases where values coming fromt the frontend are 'heavy', we might want to opt out + # in cases where values coming from the frontend are 'heavy', we might want to opt out class ValueWidget(Widget): - value = Float().tag(sync=True, no_echo=True) + value = Float().tag(sync=True, echo_update=False) widget = ValueWidget(value=1) assert widget.value == 1 @@ -358,4 +369,4 @@ class ValueWidget(Widget): # a regular set should sync to the frontend widget.value = 43 - widget._send.assert_has_calls([mock.call({'method': 'update', 'state': {'value': 43.0}, 'buffer_paths': [], 'echo': ['value']}, buffers=[])]) + widget._send.assert_has_calls([mock.call({'method': 'update', 'state': {'value': 43.0}, 'buffer_paths': []}, buffers=[])]) diff --git a/python/ipywidgets/ipywidgets/widgets/widget.py b/python/ipywidgets/ipywidgets/widgets/widget.py index 44baa135da..a57d45a8da 100644 --- a/python/ipywidgets/ipywidgets/widgets/widget.py +++ b/python/ipywidgets/ipywidgets/widgets/widget.py @@ -5,7 +5,6 @@ """Base Widget class. Allows user to create widgets in the back-end that render in the Jupyter notebook front-end. """ -import ast import os from contextlib import contextmanager from collections.abc import Iterable @@ -19,9 +18,22 @@ from base64 import standard_b64encode from .._version import __protocol_version__, __control_protocol_version__, __jupyter_widgets_base_version__ + +# Based on jupyter_core.paths.envset +def envset(name, default): + """Return True if the given environment variable is turned on, otherwise False + If the environment variable is set, True will be returned if it is assigned to a value + other than 'no', 'n', 'false', 'off', '0', or '0.0' (case insensitive). + If the environment variable is not set, the default value is returned. + """ + if name in os.environ: + return os.environ[name].lower() not in ['no', 'n', 'false', 'off', '0', '0.0'] + else: + return bool(default) + PROTOCOL_VERSION_MAJOR = __protocol_version__.split('.')[0] CONTROL_PROTOCOL_VERSION_MAJOR = __control_protocol_version__.split('.')[0] -JUPYTER_WIDGETS_ECHO = bool(ast.literal_eval(os.environ.get('JUPYTER_WIDGETS_ECHO', '1'))) +JUPYTER_WIDGETS_ECHO = envset('JUPYTER_WIDGETS_ECHO', default=True) def _widget_to_json(x, obj): if isinstance(x, dict): @@ -417,12 +429,10 @@ def get_view_spec(self): def _default_keys(self): return [name for name in self.traits(sync=True)] - _property_lock = Dict() # only used when JUPYTER_WIDGETS_ECHO=0 + _property_lock = Dict() _holding_sync = False - _holding_sync_from_frontend_update = False _states_to_send = Set() _msg_callbacks = Instance(CallbackDispatcher, ()) - _updated_attrs_from_frontend = None #------------------------------------------------------------------------- # (Con/de)structor @@ -501,15 +511,6 @@ def send_state(self, key=None): """ state = self.get_state(key=key) if len(state) > 0: - if JUPYTER_WIDGETS_ECHO: - state, buffer_paths, buffers = _remove_buffers(state) - msg = {'method': 'update', 'state': state, 'buffer_paths': buffer_paths} - if self._updated_attrs_from_frontend: - msg['echo'] = self._updated_attrs_from_frontend - self._updated_attrs_from_frontend = None - self._send(msg, buffers=buffers) - return - if self._property_lock: # we need to keep this dict up to date with the front-end values for name, value in state.items(): if name in self._property_lock: @@ -562,16 +563,21 @@ def _compare(self, a, b): def set_state(self, sync_data): """Called when a state is received from the front-end.""" + # Send an echo update message immediately if JUPYTER_WIDGETS_ECHO: - with self._hold_sync_frontend(), self.hold_trait_notifications(): - # keep this as a list, not a set, since that preserves order (useful in the test) - self._updated_attrs_from_frontend = [name for name in sync_data if name in self.keys] - for name in sync_data: - if name in self.keys: - from_json = self.trait_metadata(name, 'from_json', - self._trait_from_json) - self.set_trait(name, from_json(sync_data[name], self)) - return + echo_state = {} + for attr,value in sync_data.items(): + if self.trait_metadata(attr, 'echo_update', default=True): + echo_state[attr] = value + if echo_state: + echo_state, echo_buffer_paths, echo_buffers = _remove_buffers(echo_state) + msg = { + 'method': 'echo_update', + 'state': echo_state, + 'buffer_paths': echo_buffer_paths, + } + self._send(msg, buffers=echo_buffers) + # The order of these context managers is important. Properties must # be locked when the hold_trait_notification context manager is # released and notifications are fired. @@ -621,18 +627,6 @@ def notify_change(self, change): # Send the state to the frontend before the user-registered callbacks # are called. name = change['name'] - if JUPYTER_WIDGETS_ECHO: - if self.comm is not None and self.comm.kernel is not None and name in self.keys: - if self._holding_sync: - # if we're holding a sync, we will only record which trait was changed - # but we skip those traits marked no_echo, during an update from the frontend - if not (self._holding_sync_from_frontend_update and self.trait_metadata(name, 'no_echo')): - self._states_to_send.add(name) - else: - # otherwise we send it directly - self.send_state(key=name) - super().notify_change(change) - return if self.comm is not None and self.comm.kernel is not None: # Make sure this isn't information that the front-end just sent us. if name in self.keys and self._should_send_property(name, getattr(self, name)): @@ -650,7 +644,9 @@ def __repr__(self): @contextmanager def _lock_property(self, **properties): """Lock a property-value pair. + The value should be the JSON state of the property. + NOTE: This, in addition to the single lock for all state changes, is flawed. In the future we may want to look into buffering state changes back to the front-end.""" @@ -696,21 +692,6 @@ def _should_send_property(self, key, value): else: return True - @contextmanager - def _hold_sync_frontend(self): - """Same as hold_sync, but will not sync back traits tagged as no_echo""" - if self._holding_sync_from_frontend_update is True: - with self.hold_sync(): - yield - else: - try: - self._holding_sync_from_frontend_update = True - with self.hold_sync(): - yield - finally: - self._holding_sync_from_frontend_update = False - - # Event handlers @_show_traceback def _handle_msg(self, msg): @@ -732,11 +713,7 @@ def _handle_msg(self, msg): # Handle a custom msg from the front-end. elif method == 'custom': if 'content' in data: - if JUPYTER_WIDGETS_ECHO: - with self._hold_sync_frontend(): - self._handle_custom_msg(data['content'], msg['buffers']) - else: - self._handle_custom_msg(data['content'], msg['buffers']) + self._handle_custom_msg(data['content'], msg['buffers']) # Catch remainder. else: diff --git a/python/ipywidgets/ipywidgets/widgets/widget_upload.py b/python/ipywidgets/ipywidgets/widgets/widget_upload.py index 74a9408619..bd0fe61c95 100644 --- a/python/ipywidgets/ipywidgets/widgets/widget_upload.py +++ b/python/ipywidgets/ipywidgets/widgets/widget_upload.py @@ -133,7 +133,7 @@ class FileUpload(DescriptionWidget, ValueWidget, CoreWidget): style = InstanceDict(ButtonStyle).tag(sync=True, **widget_serialization) error = Unicode(help='Error message').tag(sync=True) value = TypedTuple(Dict(), help='The file upload value').tag( - sync=True, no_echo=True, **_value_serialization) + sync=True, echo_update=False, **_value_serialization) @default('description') def _default_description(self):