-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Partial updates #680
Partial updates #680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💃 Looks like a good solution to me. I'm good with the name – another suggestion could be something like disabled_update
, or prevented_update
, but I'm not crazy about those either! Don't let the naming block you too much I guess. :-)
component_ids[o_id][o_prop] = val | ||
|
||
if not has_update: | ||
raise exceptions.PreventUpdate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use flask.abort
instead of going the slow exception way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what flask.abort
does, but I wanted to trigger the PreventDefault
handler, in case the renderer does any optimizations with that specific response to maximize performance.
Lines 180 to 183 in bb5ed70
@self.server.errorhandler(exceptions.PreventUpdate) | |
def _handle_error(_): | |
"""Handle a halted callback and return an empty 204 response""" | |
return '', 204 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an empty 204 which would be the same as flask.abort(204)
. Raising the exception takes longer because it has to be caught and then the handler is called whereas abort is immediate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually seems like flask.abort
is doing more than that... if I try to do flask.abort(204)
it fails with LookupError: no exception for 204
. The docs say:
If a status code is given it’s looked up in the list of exceptions and will raise that exception
Which anyway sounds at least as complicated as what we're doing with raise PreventUpdate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, 204 is not an error. But then you are at the dispatch level, think you can just do return '', 204
and avoid any error handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're in the weeds now - the return from this function feeds into response.set_data()
in the actual dispatch
, which I don't believe accepts a status code.
Line 1094 in b2a97b4
response.set_data(self.callback_map[output]['callback'](*args)) |
There may be a way to improve on this but I'd like to defer it for this PR, if that's alright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that's new. Anyway, this works well for partial updates.
With this partial update logic if a callback outputs to 2 props "foo" and "bar" where "foo" receives an update but "bar" doesn't then I assume if "bar" is the input to another callback then that callback doesn't get triggered? |
@notatallshaw I believe that's how it works - I should probably add another test to 🔒 this down though. (we do have tests confirming that this is what happens with |
if val is not no_update: | ||
has_update = True | ||
o_id, o_prop = o.component_id, o.component_property | ||
component_ids[o_id][o_prop] = val |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad that we're returning a dict instead of a list here 😸 I was sure that we would've had to change the renderer to do this.
At first glance, I sort of like But I also like the symmetry of And now that I think about it, with So, 👍 💃 💃 💃 |
Fixes #668
Provides a new
dash.no_update
- a singleton that you can use in a callback return to prevent a single output from updating. Two reasons you might want to do this:I'm not super excited about the name, any suggestions?
PreventUpdate
makes sense for an exception as it's based on a verb, this seems like it should be a noun but related toPreventUpdate
, so I went withno_update
.Also its location: it's not an exception so doesn't seem to belong as
dash.exceptions.no_update
. For now I just put it at the top level asdash.no_update
. OK?