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 kernel path for thebelab #189

Merged
merged 6 commits into from
Dec 4, 2019

Conversation

joergbrech
Copy link
Contributor

This should hopefully fix #107, now that executablebooks/thebe#114 is fixed.

It doesn't really make sense to merge this until a new Thebelab version (>0.4.0) is released and this can be tested, so I added a [WIP] label to the PR name for now.

@choldgraf
Copy link
Member

woot! this looks nice - let's give it a whirl once the new thebelab is up!

@choldgraf
Copy link
Member

Just a note on this - it looks like 0.4 is now on unpkg so can we test this?

@joergbrech
Copy link
Contributor Author

I think 0.4.0 has been out for a while now and it doesn't include the changes yet...we need to wait for the next version >0.4.0

@choldgraf
Copy link
Member

ok, 0.5.1 is now out, so this should have fixed things! I fixed conflicts and updated the code a bit to be cleaner...but it should work now, can somebody confirm?

@joergbrech
Copy link
Contributor Author

I will check next week!

@choldgraf choldgraf added the enhancement New feature or request label Nov 4, 2019
@joergbrech
Copy link
Contributor Author

Ok, so I tested this on my site (how would you unit-test this?), and the generated html looks good:

<script type="text/x-thebe-config">
    {
      requestKernel: true,
      binderOptions: {
        repo: 'joergbrech/Modellbildung-und-Simulation',
        ref: '2-thebelab-directory',
      },
      codeMirrorConfig: {
        theme: "abcdef"
      },
      kernelOptions: {
        name: 'octave',
        path: "content/00_einleitung"
      }
    }
</script>

Running thebelab uses the specified path, so this seems to work well, but there is still a problem:

kernelOptions.name is not respected. A python kernel is started and not an octave kernel. Opening the Jupyter Notebook on binder works fine.

@choldgraf
Copy link
Member

interesting - so kernelOptions.path is respected, but kernelOptions.name is not?

for unit testing, that's a great question :-) I have struggled with "how to unit test javascript + html" since the beginning of jupyter book. I think there are frameworks for doing this but none that I've had the time to learn and implement

@joergbrech
Copy link
Contributor Author

Seems like it. When pressing the thebelab button, the console prints

Binder: server running at https://hub.gke.mybinder.org/user/joergbrech-mode--und-simulation-i1emjsxk/
thebelab.js:416:16
Starting WebSocket: wss://hub.gke.mybinder.org/user/joergbrech-mode--und-simulation-i1emjsxk/api/kernels/4bbd8147-e749-46ff-9775-231c4ebb1b32 default.js:51:27
Kernel: connected (4bbd8147-e749-46ff-9775-231c4ebb1b32)

and when opening the url at https://hub.gke.mybinder.org/user/joergbrech-mode--und-simulation-i1emjsxk/api/kernels to check for the running kernel I get the response

[{"id": "4bbd8147-e749-46ff-9775-231c4ebb1b32", "name": "python3", "last_activity": "2019-11-05T06:12:15.829162Z", "execution_state": "idle", "connections": 1}]

So apparently a python3 kernel is started.

And since thebelab is running with a python kernel instead of an octave kernel, I can copy

import os
print(os.getcwd())

into any of the code cells to verify that binder is running in the correct path.

I will try and dig deeper, but this new(ish) kernel issue might be unrelated to #107 and this PR...

for jupyterlab sessions API

addresses executablebooks#107
@joergbrech
Copy link
Contributor Author

joergbrech commented Nov 5, 2019

Found it! It turns out we should use kernelName and not name, since thebelab now uses the session API and not the kernel API:

I am not sure why this test fails?

@joergbrech joergbrech changed the title [WIP] Add kernel path for thebelab Add kernel path for thebelab Nov 5, 2019
@choldgraf
Copy link
Member

This LGTM, thanks very much for the fix @joergbrech !!

@choldgraf choldgraf merged commit 94b5d92 into executablebooks:master Dec 4, 2019
@choldgraf choldgraf added the 🏷️ maintenance Generic under-the-hood improvements to code and docs label Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🏷️ maintenance Generic under-the-hood improvements to code and docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

thebelab always runs in home directory
2 participants