Add concept of block events and some design notes #1059
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1059 +/- ##
==========================================
+ Coverage 70.14% 70.42% +0.27%
==========================================
Files 63 63
Lines 4107 4166 +59
==========================================
+ Hits 2881 2934 +53
- Misses 1226 1232 +6
🚀 New features to boost your workflow:
|
datalab
|
||||||||||||||||||||||||||||
| Project |
datalab
|
| Branch Review |
ml-evs/blocks
|
| Run status |
|
| Run duration | 07m 10s |
| Commit |
|
| Committer | Matthew Evans |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
471
|
| View all changes introduced in this branch ↗︎ | |
ml-evs
force-pushed
the
ml-evs/blocks
branch
2 times, most recently
from
2f892ca to
f765a97
Compare
|
Just pinging @BenjaminCharmes and @jdbocarsly for review on this one! |
| @@ -27,9 +27,19 @@ class XRDBlock(DataBlock): | |||
| def plot_functions(self): | |||
| return (self.generate_xrd_plot,) | |||
|
|
|||
| @classmethod | |||
| @event | |||
| def set_wavelength(self, wavelength: float | None): | |||
There was a problem hiding this comment.
Can you show how this gets implemented in the XRD block before we merge? That would be a good way to test that everything is working on the front end
| @@ -207,6 +207,10 @@ export default { | |||
| this.contentMaxHeight = "none"; | |||
| } | |||
| }); | |||
| document.addEventListener("bokehStateUpdate", this.handleBokehEvent); | |||
There was a problem hiding this comment.
If this adds a listener to the overall document, will we have multiple listeners if we have multiple blocks on a page? This could cause the handler to be called multiple times
you should be able to add this listener to a only the relevant node by assigning the outer div ref="thisBlock", then only add the listener to:
this.$refs.thisBlock.addEventListener("bokehStateUpdate", this.handleBokehEvent);
|
Made some fixes post-discussion with @jdbocarsly. Known limitations still:
|
This PR adds the concept of "events" that can be registered on block types; essentially, server-side functions that can be triggered separately from parsing/ingestion for things like setting parameters. It also introduces the handling of particular events raised by Bokeh plots in the base data block in Vue.
This is all described in a new design document for this current version of datablocks, also committed in this PR.
I think eventually we can add several lifecycle hooks to the base block for handling certain things (remake block from scratch etc) that can be reused by derived block types, though I haven't put any effort into doing this now. I'm already using this functionality in a custom block that sets some parameters based on user feedback that requires reanalysing all block data (essentially a slider setting some offset time parameter).
For review, I recommend reading the design doc in the rtd preview at https://the-datalab--1059.org.readthedocs.build/en/1059/design/blocks/.
There's still a few open questions about how errors in events should be handled, and what methods should be triggered afterwards (mostly due to the fact we always call the
to_web()block method in the update-block route.