Timing issue between importing dependency and making use of it about declarativewidgets

[lbustelo]

We need to see if there is a way to prevent the cell execution to continue until we are done with loading the links.

[parente]

FWIW, this problem exists (existed? we're not on 4.x line of code) in ipywidgets too. The scotch and omnibus demos both suffer from race conditions sometimes when run all is used. In that case, there's a race between the widget JS view finishing eval when echoed back to the notebook frontend in a %%javascript cell and the execution of the widget model cell that tries to instantiate the frontend view.

[jhpedemonte]

Well, shouldn't the notebook just add the HTML/Javascript to the DOM and let the browser handle it? If it does, then I think Polymer would do the right thing (in this case).

Of course, there are issues (mainly perf) with writing to the DOM cell-by-cell. Another approach may be to update the Notebook code to simply aggregate HTML/Javascript cells (in order) when going cell-by-cell, then do one DOM write at the end of the page.

[parente]

This one is starting to become a pain in demos where we have to inject arbitrary sleep statements (ugh) to ensure everything finishes bower installing in the back before code in the front tries to use it. I don't think it's solely related to bower install timing either: I've seen deployed dashboards fail to render properly because of the asynchronicity of having HTML linked assets hit the glass vs cell executions further down a notebook/dashboard page try to use them.

In the notebook environment, here's an educated guess about why this seems to work sometimes and not others. Because the installs are happening synchronously (issue #43), the Tornado server remains blocked during a bower install and prevents messages from getting to the kernel by chance. But I think if you have more than one link import in a cell, those are done as separate AJAX requests from the frontend, so there are gaps that allow kernel execution requests which are queued up from the rest of the notebook to sneak through between the various bower installs.

I thought about the following workaround: put one link tag per cell and only that link tag in the cell. It does make things a bit better, but it too has a subtle race condition. In this setup, you can't have later cell execution requests sneaking through between import calls. But you still have a race between the HTML getting echoed back to the frontend, the link logic trying to fetch the widget HTML (which doesn't exist), and the link import logic hitting the server import endpoint (which freezes further execution) to lock the kernel again.

@lbustelo I'm open to brainstorming on this one.

[lbustelo]

@parente Yes! Let's brainstorm today. A couple of things to consider.

1. Webcomponents should be capable of getting promoted after creation time.
2. I think I read code where templates are supposed to wait for imports to be completed for all the children elements.

Are for your guess of why it works on the notebook, I think you are spot on. And I suspect that once we find a fix #43, we will see the same problems in the notebook.

Gino B.

On Oct 20, 2015, at 7:25 AM, Peter Parente [email protected] wrote:

This one is starting to become a pain in demos where we have to inject arbitrary sleep statements (ugh) to ensure everything finishes bower installing in the back before code in the front tries to use it. I don't think it's solely related to bower install timing either: I've seen deployed dashboards fail to render properly because of the asynchronicity of having HTML linked assets hit the glass vs cell executions further down a notebook/dashboard page try to use them.

In the notebook environment, here's an educated guess about why this seems to work sometimes and not others. Because the installs are happening synchronously (issue #43), the Tornado server remains blocked during a bower install and prevents messages from getting to the kernel by chance. But I think if you have more than one link import in a cell, those are done as separate AJAX requests from the frontend, so there are gaps that allow kernel execution requests which are queued up from the rest of the notebook to sneak through between the various bower installs.

I thought about the following workaround: put one link tag per cell and only that link tag in the cell. It does make things a bit better, but it too has a subtle race condition. In this setup, you can't have later cell execution requests sneaking through between import calls. But you still have a race between the HTML getting echoed back to the frontend, the link logic trying to fetch the widget HTML (which doesn't exist), and the link import logic hitting the server import endpoint (which freezes further execution) to lock the kernel again.

@lbustelo I'm open to brainstorming on this one.

—
Reply to this email directly or view it on GitHub.

[parente]

Well, it doesn't work in the notebook today. The sleep is needed there too because of the execute leaks. The fix for #43 will make it worse because then there would be no blocking whatsoever.

[lbustelo]

I guess I mean to say "sort of works!". I'm aware of the need for the sleep
in the cell... But even that is not a 100% solution.

On Tue, Oct 20, 2015 at 7:48 AM, Peter Parente [email protected]
wrote:

Well, it doesn't work in the notebook today. The sleep is needed there
too because of the execute leaks. The fix for #43
#43 will
make it worse because then there would be no blocking whatsoever.

—
Reply to this email directly or view it on GitHub
#25 (comment)
.

[parente]

Yes. The sleep is a hack. The only correct fix is that the frontend load/execute has to sync with the backend execute/respond somehow.

Let's chat this PM.

[jhpedemonte]

Looks like this is specifically related to defining a Polymer element 'inline'. The recommendation there is to use HTMLImports.whenReady() callback before defining. When I update the test notebook linked in the description, it works -- I just need to also load webcomponents-lite.js. Will test with a larger notebook now.

[jhpedemonte]

Actually, use of HTMLImports.whenReady() only works if the elements have already been bower installed. If the page has to wait for bower install, then the toggle button is once again broken.

[lbustelo]

Was the observation that the whenReady callback was getting called before the bower install portion is done?

[jhpedemonte]

Tested this on a larger (non-test) notebook, which contains a Polymer definition similar to that of the test notebook attached in this issue. I (1) added <script> to bring in webcomponents-lite.min.js, (2) commented out time.sleep(), and (3) added HTMLImports.whenReady() in the Polymer element definition.

When running in new container, server kicks off bower install of components. When done, some Polymer elements on the page load fine, but not our custom element defined in the page. Even re-running the cell doesn't show the element. Did whenReady not fire?

But then I refresh the page and run all again and now the custom element loads fine and its toggle button functions normally.

So HTMLImports is a half-fix. Still issues around bower install.

[parente]

With latest fix, I haven't seen any further issue with deployed dashboards after removing the hacky sleeps. The problem now only surfaces on the first Run All of a notebook that requires bower install as @jhpedemonte said.

All signs point to the 404 -> install -> 200 trick as being the culprit but no concrete proof.

Throwing this out there even though I don't think it'll work...

What if on the first fetch attempt from the urth_components handler, instead of returning a 404 if the file does not exist, it runs off, does the bower install, and then completes the request? The browser then only sees a 200, albeit a slow one when the install needs to occur. Problem with this is how long will a browser wait before giving up on loading the target?

[purdrew]

404 -> install -> 200 trick won't work with the current implementation. We are currently overloading the link tag, so the initial request will just be for a file path (ie. urth_components/paper-slider/paper-slider.html). There is no additional information sent with the request, like the bower package name. Without the bower package name there is no reliable means of inferring what package needs to be installed. I previously investigated trying to hook into the initial file request but was unable to determine a way to override it.

I think the only way to do a 404 -> install -> 200 trick would be to create are own custom element and not overload the link tag. That way we could manage the initial lookup. Thoughts?

[parente]

To be clear, I was calling the current approach the 404 -> install -> 200 trick and saying that might be the culprit for the problem. So I must be missing what you're saying with "404 -> install -> 200 trick won't work with the current implementation." I thought it is the current impl?

[purdrew]

Sorry, meant we can't internally trap the first 404 on the server side and instead of returning the 404 to the browser, go ahead and bower install then return success.

[parente]

Stepping back, looks like the "install if it doesn't exist, load if it does" magic here might be impossible if we want things to work properly. (IMHO, working correctly every time is more important than magic. Reloads on bad wifi on Jupyter Day was painful because of this.)

What if:

1. To get a package, user has to run a code cell in a notebook that explicitly installed it. To be kernel language neutral, let's pretend it's in JS or a <urth-import package="blah"> element.
2. Once the package is installed, then the user can include a link element pointing at it like he/she does today.
3. If the user points the link at it before installing, it does nothing / gets the 404.

This separates the install from the use. It (hopefully) avoids the polymer getting confused because of the 404 hook (we think), perhaps even in case #3. And it's no different than the install-before-use requirement for dependences on the kernel-side (e.g., in Python yo have to pip install something, then import it, then use it; same with Scala, R, Julia, ...).

[lbustelo]

With all the troubles that we've been having with extending the <link> element, I think it makes sense to reconsider our approach. I do like the idea of a clean separation:

1. A new urth element just for bower installation, <urth-core-install>
2. The use of a regular <link rel=import> element without any urth extensions.
3. We remove is=urth-core-import extension

Open questions:

1. Is having the /urth_import handler be blocking (only for other /urth_import calls) enough to garantee that <link> elements in cells below will execute after all installs defined above are complete?

Also, this <urth-core-install> element needs to be a noop while in dashboard mode.

[jhpedemonte]

Will this approach work with "Run all"? Is this what you had in mind with your open question?

[lbustelo]

@jhpedemonte yes. Run all is the litmus test.

[parente]

Is having the /urth_import handler be blocking (only for other /urth_import calls) enough to garantee that elements in cells below will execute after all installs defined above are complete?

Hypothesis: If there's exactly one import statement per cell, then it'll work. If there's multiple, then the server will free up between import calls and allow further queued cell execute requests to reach the kernel. If one of those queued requests is an %%html <link> then it's going to 404.

Or not. This stuff is really hard to reason about, so it's probably worth trying it as an experiment and seeing what happens in practice.

[purdrew]

I'm failing to see how the "separate install from import" suggestion actually solves the run all problem. Seems like we'd still be in the situation of having to run an install cell prior to all the other cells. So that is no different then the current situation.

[parente]

It's been a few days, but I thought the point was to avoid the 404 -> install -> 200 problem screwing up polymer in a weird way. The import would still need to be made to block further code executions against the kernel somehow, yes. And I have no idea how to do that.

[parente]

Before I forget, my thinking on how to proceed after our last discussion:

1. KISS. Require a user to run bower install <opts> <list of deps> or bower install <opts> package.json to get what needs to be installed, installed properly into the global urth_components bower root. This is no different than telling users to run pip, conda, install.packages, etc. ahead of import, library, etc. It's also the problem tools like Binder are trying to solve by dynamically building reproducible environments.
2. Once that baseline is working, look at optimizations that make install and link possible all within the notebook, but not necessarily all within a single tag.

[zackmorris]

@lbustelo and I took another look at this (the remote branch is here).

We created an import widget that sends the POST request to the server by modifying urth-core-import to send the specified package through the import widget. While this was a successful change, it doesn't block the kernel from executing every cell when using Run All - content is rendered on the page as soon as it's finished installing on the server.

[lbustelo]

Let me elaborate a bit more on above...

1. We modified urth-core-import to be a hybrid widget (i.e have a kernel side proxy).
2. urth-core-import would then send the information about the packge to install to the kernel side using COMM.
3. The actual POST to the /import URL on the NB Server would then be done from the kernel and block other kernel execs
4. When the kernel detects that the import is done, it will then notify the urth-core-import element and the link tag get added.

The idea of this approach was to do the install work from the kernel to be able to sequence the work and block cell execution. What we did not account for was how Run-all actually work. It will queue up all the cell executions before our urth-core-import elements would start sending COMM message to install packages.

[purdrew]

The example notebook is working for me (with fixed href path of "urth_components/.....") on Chrome, Firefox and Safari on latest code with Polymer 1.3.

[lbustelo]

I'm closing this issue. I think this works now as good as it can. The late stamping of urth-core-bind based on imports #215 is a great solution to many of the Run All issues.