Skip to content
This repository has been archived by the owner on Feb 4, 2021. It is now read-only.

Add JupyterLab support #41

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add JupyterLab support #41

wants to merge 5 commits into from

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented May 11, 2018

Fix #39.

This adds a Spark status window to the side pane in Jupyter Lab. I played with making a modal dialog like the old extension, but it feels like the side pane is more in keeping with the "JupyterLab way".

It looks like:

image

@teonbrooks, @jezdez

@mdboom mdboom requested a review from jezdez May 11, 2018 21:52
@codecov-io
Copy link

codecov-io commented May 11, 2018

Codecov Report

Merging #41 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #41   +/-   ##
=======================================
  Coverage   96.61%   96.61%           
=======================================
  Files           3        3           
  Lines          59       59           
  Branches        5        5           
=======================================
  Hits           57       57           
  Misses          2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34ab4bf...430f619. Read the comment docs.

Copy link
Contributor

@jezdez jezdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor fixes needed, LGTM otherwise. Thank you @mdboom!

README.md Outdated
## Server

The server that communicates between the Jupyter server and Spark is the same
regardless of the frontend used. It wueries the Spark UI service on the backend
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

"@phosphor/disposable": "^1.1.2",
"jquery": "^3.3.1",
"bootstrap": "^4.1.1"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

id: 'jupyter_spark',
autoStart: true,
activate: (app) => {
let api_url = "/spark/api/v1";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past we had trouble with the base path for the API, are you sure that's not the case here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder. Indeed it looks like we have the same problem here. I'll fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed now.

@mdboom
Copy link
Contributor Author

mdboom commented May 14, 2018

We'll want to deploy this to npm somehow -- ideally using a Travis deploy clause like we already do for PyPI.

@AbdealiLoKo
Copy link

I came here looking for this exact feature - glad to see it already has a PR!
A quick thought: Would be good to keep the progress bar as a second row as the sidebar is not meant for long horizontal outputs.

Current look:

Job ID1       Job Name      Progress
Job ID2       Job Name      Progress
Job ID3      Job Name      Progress

What I'm suggesting:
Current look:

Job ID1       Job Name
Progress ============>
Job ID2       Job Name
Progress ============>
Job ID3      Job Name
Progress ============>

Is there also a progress bar at the cell that is running ?

@mdboom
Copy link
Contributor Author

mdboom commented May 24, 2018

That's a good suggestion about the layout of the table. It shouldn't be too difficult to make that change.

There isn't a progress bar at the cell that is running (unlike in the "classic notebook" version of this plugin), because if I understand correctly, the API for that isn't released yet (http://jupyterlab.readthedocs.io/en/stable/developer/notebook.html#the-ipywidgets-third-party-extension). But I honestly didn't do too much digging about it, so if there's an obvious way forward on that that isn't likely to change too much down the road, I'm game.

@elehcimd
Copy link

Would be great to get this merged, any blockers?

@jezdez
Copy link
Contributor

jezdez commented Jan 17, 2019

@elehcimd The problem is mostly making the table changes as @mdboom replied above and figure out a way how to deploy this to npm (and how that integrates with the production deployment of Jupyter).

@mdboom Would you mind elaborating how we'd apply this to our Spark instance? Would we have to install Node and run the full NPM build to get it to run in our lab environment?

@mdboom
Copy link
Contributor Author

mdboom commented Jan 17, 2019

@jezdez: Yes, Jupyter lab extensions have to go through a build process which involves a local copy of node. Details here

@AlJohri
Copy link

AlJohri commented Apr 23, 2019

gentle bump. would love to see some version of this integrated

@IMAM9AIS
Copy link

IMAM9AIS commented Jul 29, 2019

@jezdez @mdboom Thanks for this extension.
We tried testing this extension on an enterprise level setup.
From what i can understand the plugin tries to go through all the applications in the spark link, and maintains a cache for. It triggers the progress bar update if it sees a job in progress for any application id.

However, if we actually start considering this solution on an enterprise level, where we have millions of applications running in multiple queues, even a first iteration of update of all applications is taking a lot of time and incase a lot of latency.

Rather than creating a list of all application ids from the cluster, is there a workaround we can have in terms of the cache the plugin maintains ?

Is it possible that we can take the application ids of only the lab .ipynb files that are opened up, and maintain a cache for that, rather than having a cache of the entire array of ids.

Do let me know if we can do this. I happy to contribute, given some guidance .

@birdsarah
Copy link

I made a PR here that updates this PR to work with modern jupyter (esp latest tornado).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question] Add progress bar to jupyter lab?
8 participants