Upgrade to support Jupyter Lab 3

jacobtomlinson commented 3 years ago

Upgrade to support Jupyter Lab 3 via the migration guide.

The primary benefit here other than just staying up to date is that the JavaScript extensions support modular prebuilt code. So whole extensions can now be distributed via pip/conda without needing nodejs or npm.

rcthomas commented 3 years ago

It looks like applying the migration tool to this labextension works pretty smoothly, but that CI issues are blocking the merge, is that correct?

jacobtomlinson commented 3 years ago

@rcthomas yeah I think so.

@raydouglass could you help me get this unblocked. I'm not sure what the CI is unhappy about.

ajschmidt8 commented 3 years ago

@jacobtomlinson, I was able to replicate the build errors from this Jenkins log locally. Ultimately, it seems the jupyter labextension build . command shown below is exiting with an error code of 1.

After running jupyter labextension build . -h, the output (below) seems to indicate that build may not be a valid subcommand. Any thoughts on this? Do you think another package may need to be installed in order to enable the build subcommand?

`jupyter labextension build . -h` output

``` Work with JupyterLab extensions Subcommands =========== Subcommands are launched as `jupyter labextension cmd [args]`. For information on using subcommand 'cmd', do: `jupyter labextension cmd -h`. install Install labextension(s) update Update labextension(s) uninstall Uninstall labextension(s) list List labextensions link Link labextension(s) unlink Unlink labextension(s) enable Enable labextension(s) disable Disable labextension(s) check Check labextension(s) Options ======= The options below are convenience aliases to configurable class-options, as listed in the "Equivalent to" description-line of the aliases. To see all configurable class-options for some , use: --help-all --debug set log level to logging.DEBUG (maximize logging output) Equivalent to: [--Application.log_level=10] --generate-config generate default config file Equivalent to: [--JupyterApp.generate_config=True] -y Answer yes to any questions instead of prompting. Equivalent to: [--JupyterApp.answer_yes=True] --log-level= Set the log level by value or name. Choices: any of [0, 10, 20, 30, 40, 50, 'DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL'] Default: 30 Equivalent to: [--Application.log_level] --config= Full path of a config file. Default: '' Equivalent to: [--JupyterApp.config_file] Examples -------- jupyter labextension list # list all configured labextensions jupyter labextension install # install a labextension jupyter labextension uninstall # uninstall a labextension To see all available configurables, use `--help-all`. ```

jacobtomlinson commented 3 years ago

Thanks for digging into this @ajschmidt8.

This PR was created mostly by an automated migration tool from Jupyter folks. Obviously that tool doesn't support migrating GPUCI scripts, so I'm guessing something is wrong in there and will need migrating by hand.

Digging into it I think we want jlpm build now.

I'll make some updates to this PR and see how it goes.

jacobtomlinson commented 3 years ago

Hrm it looks like jupyter labextension build was added by the migration tool.

This package is very similar to the dask-labextension so I'm going to try updating the conda meta to be a little more in line with the feedstock for that project.

jacobtomlinson commented 3 years ago

@rcthomas do you have any thoughts on why we are seeing this error about jupyter labextension build?

rcthomas commented 3 years ago

@jacobtomlinson No I don't, let me page @marius311 here who I know has built and tested the labextension himself. If the call to labextension build was added by the migration tool I think he would have hit it since he informed me that the tool was all he needed to use.

marius311 commented 3 years ago

let me page @marius311 here who I know has built and tested the labextension himself

Correct, my exact sequence of commands was

git clone <this repo>
python -m jupyterlab.upgrade_extension .
pip install -e .
jlpm install
jlpm run build
jupyter labextension install .

and it works.

This is the output of jupyter labextension build if it helps in any way, but I probably don't know enough to help beyond that.

$ jupyter labextension build
Building extension in /global/u1/m/marius/src/jupyterlab-nvdashboard

Compilation starting…

Compilation finished

asset 568.eefb815965ad9d435529.js?v=eefb815965ad9d435529 9.52 KiB [emitted] [immutable] [minimized]
asset remoteEntry.849945296e2fd9727bd4.js 6.55 KiB [emitted] [immutable] [minimized] (name: jupyterlab-nvdashboard)
asset 534.706d853a65bc86cdc1b8.js?v=706d853a65bc86cdc1b8 3.34 KiB [emitted] [immutable] [minimized]
runtime modules 17.6 KiB 12 modules
orphan modules 325 bytes [orphan] 1 module
built modules 20.7 KiB (javascript) 336 bytes (consume-shared) 42 bytes (share-init) [built]
  javascript modules 20.7 KiB
    modules by path ./style/ 4.5 KiB 6 modules
    modules by path ./node_modules/ 9.04 KiB 3 modules
    modules by path ./lib/*.js 7.06 KiB 2 modules
    container entry 42 bytes [built] [code generated]
  consume-shared-module modules 336 bytes
    modules by path consume shared module (default) @jupyterlab/ 168 bytes 4 modules
    modules by path consume shared module (default) @lumino/ 84 bytes 2 modules
    modules by path *.1 (singleton) 84 bytes
      consume shared module (default) react@^17.0.1 (singleton) 42 bytes [built] [code generated]
      consume shared module (default) react-dom@^17.0.1 (singleton) 42 bytes [built] [code generated]
  provide shared module (default) jupyterlab-nvdashboard@0.5.0 = ./lib/index.js 42 bytes [built] [code generated]
webpack 5.28.0 compiled successfully in 3205 ms

jacobtomlinson commented 3 years ago

Thansk @marius311. Any idea why we are seeing errors in CI that build is not a valid subcommand of jupyter labextension? Are we missing some dependency?

tonyx93 commented 3 years ago

Hey folks, just spitballing here but maybe the reason why the CI can't find the build subcommand is because it needs to have JupyterLab 3 installed? From the logs, it seems like it still has JupyterLab 2.X

jacobtomlinson commented 3 years ago

It looks like the automatic migration tool added some GitHub Actions stuff. I've tried to roll that into the GPUCI scripts.

I've also tried to migrate the packaging over following the docs on this page.

I've successfully managed to build the wheel locally and then install it into a fresh conda environment and everything works as expected. So no more NPM packging 🎉.

I'm not 100% sure if anything needs changing in the conda recipe or if things are correct in the CI scripts.

@ajschmidt8 would you mind reviewing?

Also @jakirkham if you have some time I would really appreciate a packaging review here. Jupyter Lab 3 changed things a bunch. Specifically that we need to use build to compile the JavaScript and bundle it, so I'm not sure how that maps over to conda recipes.

ajschmidt8 commented 3 years ago

@jacobtomlinson, I updated a few things in CI here and fixed some linting errors. The jupyter serverextension list 2>&1 | grep -ie "jupyterlab_nvdashboard.*OK" command here is currently exiting with a non-zero exit code. I've commented it out in a2b5251. Can it be removed? The jupyter labextension list command below it passes. I'm not too familiar with this plugin/repo, but I'm wondering if it's a labextension and not a serverextension and that's why the command fails.

jacobtomlinson commented 3 years ago

Thanks for the review @jakirkham.

Thanks @ajschmidt8, this extension has both a serverextension and a labextension so both checks should pass.

jacobtomlinson commented 3 years ago

Actually @ajschmidt8 I think you're right. The server-side component in this extension uses the jupyter_serverproxy_servers entrypoint to start the server component, not a server extension.

jacobtomlinson commented 3 years ago

Ok I think this is ready for a merge. @ajschmidt8 could you take one last look?

tonyx93 commented 3 years ago

Are there any updates with this PR?

ajschmidt8 commented 3 years ago

rerun tests

ajschmidt8 commented 3 years ago

Are there any updates with this PR?

@txu0393, assuming CI passes during this next run, we should be all set to merge.

Atharex commented 3 years ago

@ajschmidt8 you might want to review the build script. It seems it's using a faulty command https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/rapidsai%2Fgpuci%2Fjupyterlab-nvdashboard%2Fprb%2Fjupyterlab-nvdashboard-cpu-build/detail/jupyterlab-nvdashboard-cpu-build/53/pipeline#log-933

  $ jupyter labextension build .
  Please supply at least one subcommand: check, disable, enable, install, link, list, uninstall, unlink, update
  error Command failed with exit code 1.

Atharex commented 3 years ago

This looks like a failure due to different jupyterlab versions...

The centos pipeline is explicitly installing jupyterlab v3 https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/rapidsai%2Fgpuci%2Fjupyterlab-nvdashboard%2Fprb%2Fjupyterlab-nvdashboard-gpu-build/detail/jupyterlab-nvdashboard-gpu-build/166/pipeline/#log-230

The failing ubuntu pipeline seems to be using only jupyterlab 2 https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/rapidsai%2Fgpuci%2Fjupyterlab-nvdashboard%2Fprb%2Fjupyterlab-nvdashboard-gpu-build/detail/jupyterlab-nvdashboard-gpu-build/166/pipeline/#log-230

Other ubuntu pipelines again explicitly install jupyterlab 3 https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/rapidsai%2Fgpuci%2Fjupyterlab-nvdashboard%2Fprb%2Fjupyterlab-nvdashboard-gpu-build/detail/jupyterlab-nvdashboard-gpu-build/165/pipeline#log-245

https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/rapidsai%2Fgpuci%2Fjupyterlab-nvdashboard%2Fprb%2Fjupyterlab-nvdashboard-gpu-build/detail/jupyterlab-nvdashboard-gpu-build/164/pipeline#log-217

https://gpuci.gpuopenanalytics.com/blue/organizations/jenkins/rapidsai%2Fgpuci%2Fjupyterlab-nvdashboard%2Fprb%2Fjupyterlab-nvdashboard-gpu-build/detail/jupyterlab-nvdashboard-gpu-build/166/pipeline/#log-230

ajschmidt8 commented 3 years ago

There was an upstream PR for CI to resolve this that was opened but never merged. I just merged it. Let's try this again.

rerun tests

jacobtomlinson commented 3 years ago

Looks like CI passed 🎉