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

Install multiple versions of same extension #6857

Merged
merged 14 commits into from Aug 15, 2019
Merged

Install multiple versions of same extension #6857

merged 14 commits into from Aug 15, 2019

Conversation

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Jul 19, 2019

This adds support for installing multiple versions of the same JupyterLab extension, fixing #6491.

jupyter labextension install \
  --pin-version-as test-1,test-2 \
  jupyterlab-test-extension@1.0 \
  jupyterlab-test-extension@2.0

jupyterlab-test-extension is a real extension I published two versions of, with different extension ID's in each, to test with https://github.com/saulshanabrook/jupyterlab-test-extension. Once you install both of them, you should get two different console logs on startup.

If we run jupyter labextension list after installing these, we see both versions:

JupyterLab v1.0.2
Known labextensions:
   app dir: /usr/local/miniconda3/envs/jupyterlab/share/jupyter/lab
        test-1 jupyterlab-test-extension v1.0.0  enabled  OK
        test-2 jupyterlab-test-extension v2.0.0  enabled  OK

And we can uninstall each separately:

jupyter labextension uninstall test-1

This PR punts on defining update semantics for pinned packages:

$ jupyter labextension update test-1
Skipping updating pinned extension 'test-1'.
$ jupyter labextension update --all
Skipping updating pinned extension 'test-1'.
Skipping updating pinned extension 'test-2'.

How?

Normally, zipped extensions are stored with the name like jupyterlab-test-extension-1.0.0.tgz. However, we need to differentiate extensions installed as aliases, without mucking with their actual
contents. So we save them instead like pin@test-1.tgz.

Then, we we go to read our packages, we know they have an alias, and can reflect that when listing extensions or updating them.

This adds support for installing multiple versions of the same
JupyterLab extension, fixing #6491.

It reuses the syntax for
[yarn add's alias command](https://yarnpkg.com/lang/en/docs/cli/add/#toc-yarn-add-alias)
so that you can do:

```bash
jupyter labextension install test-1@npm:jupyterlab-test-extension@1.0
jupyter labextension install test-2@npm:jupyterlab-test-extension@2.0
```

This is a real extension I published two versions of, with different
extension ID's in each, to test with
https://github.com/saulshanabrook/jupyterlab-test-extension.
Once you install both of them, you should get two different console
logs on startup.

If we run `jupyter labextension list` after installing these, we see
both versions:

```bash
JupyterLab v1.0.2
Known labextensions:
   app dir: /usr/local/miniconda3/envs/jupyterlab/share/jupyter/lab
        test-1 jupyterlab-test-extension v1.0.0  enabled  OK
        test-2 jupyterlab-test-extension v2.0.0  enabled  OK
```

And we can uninstall each separately:

```bash
jupyter labextension uninstall test-1
```

As well as update each of them:

```bash
jupyter labextension update test-1
```

## How?

Normally, zipped extensions are stored with the name like
`jupyterlab-test-extension-1.0.0.tgz`. However, we need to differentiate
extensions installed as aliases, without mucking with their actual
contents.So we save them instead like
`test-1@npm:jupyterlab-test-extension-1.0.0.tgz`.

Then, we we go to read our packages, we know they have an alias.
@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented Jul 19, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 19, 2019

I haven't started implementing this yet, but I have sketched out a plan in this PR description. If anyone has comments or suggestions on the proposal, that would be great. I plan to work on this next week. cc @vidartf @blink1073

@blink1073
Copy link
Member

@blink1073 blink1073 commented Jul 20, 2019

Sounds reasonable to me!

@saulshanabrook saulshanabrook self-assigned this Jul 22, 2019
@vidartf
Copy link
Member

@vidartf vidartf commented Jul 23, 2019

I would love to give my opinion on this, but I'm afraid I don't really understand the motivation for it. Do you have a use case or two to document what problem this solves (i.e. why would you want to install two versions of the same extension)?

That said, having the syntax based on that of yarn add makes it seem plausible, although it's likely to make the build system even more complex 😩

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 23, 2019

I would love to give my opinion on this, but I'm afraid I don't really understand the motivation for it. Do you have a use case or two to document what problem this solves (i.e. why would you want to install two versions of the same extension)?

The use case is like a mimerender extension that wraps an underlying Javascript library that changes regularly. You have notebooks output a versioned mimetype, and want users to be able to view old notebooks as well as new ones. You could create a new NPM package for each version of the extension, which is what we do currently for vega. However, this is a lot of extra work on the maintainers part, especially when they don't really care about updating the old versions, they are more relics of a past state. So with this change, they can just release a new NPM version for each mime type change (making sure to change the extension ID as well). Then users can install any combinations of versions they want.

That said, having the syntax based on that of yarn add makes it seem plausible, although it's likely to make the build system even more complex 😩

Yep...

My dream is eventually we can just have labextension install xxx default to yarn install xxx; jupyter labextension enable xxx and remove most of our custom install logic.

@vidartf
Copy link
Member

@vidartf vidartf commented Jul 23, 2019

The use case is like a mimerender extension that wraps an underlying Javascript library that changes regularly.

Thanks for the example, that makes it easier to understand! In terms of solving that problem, I'm fine with the suggested approach, but I have some concerns about asking users to run such convoluted installs commands.

PS: What would happen if someone tries to use an alias equal to the name of a core package?

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 23, 2019

I have some concerns about asking users to run such convoluted installs commands.

Another option would be to just allowing users to have multiple versions installed of the same extension, like:

jupyter labextension install jupyterlab-test-extension@1.0
jupyter labextension install jupyterlab-test-extension@2.0

Except now updating or removing the extension also requires specifying a version. Whereas in the yarn alias syntax you put a name, so that you can refer to it when updating or removing.

What would happen if someone tries to use an alias equal to the name of a core package?

It probably should fail? If the alias is equal to any existing installed extensions.

@vidartf
Copy link
Member

@vidartf vidartf commented Jul 23, 2019

Another option would be to just allowing users to have multiple versions installed of the same extension

How about adding a flag to the install command?

jupyter labextension install --multiversion jupyterlab-test-extension@2.0

Maybe even an alias flag? (or some combination?)

jupyter labextension install --alias test-extension-2 jupyterlab-test-extension@2.0
jupyter labextension update test-extension-2

Anything that would make it easier for the user to understand what is going on would be a step in the right direction from my point of view. This is a little dense (smells like black magic):

jupyter labextension install my-somewhat-long-alias@npm:@my-scope/my-somwhat-long-packagename@2.0

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 23, 2019

Maybe even an alias flag? (or some combination?)

jupyter labextension install --alias test-extension-2 jupyterlab-test-extension@2.0

That looks OK to me. Seems more readable!

This is a little dense (smells like black magic):

Yep, but at least yarn documents it! So it's a quasi standard...

@vidartf
Copy link
Member

@vidartf vidartf commented Jul 23, 2019

Yep, but at least yarn documents it! So it's a quasi standard...

Which would make this ok to use if only extensions devs were exposed to it, but I would avoid exposing end users to it, which might just be starting to learn programming.

@vidartf
Copy link
Member

@vidartf vidartf commented Jul 23, 2019

Another thing that might be worth thinking through before starting work on this is what you/users would expect to happen when the update command is run:

jupyter labextension update --all

You would likely need some sort of config logic to semver-limit each alias / side-by-side install.

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 23, 2019

jupyter labextension update --all

Hm, we could just skip aliased packages when running update --all?

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 24, 2019

We had a little discussion about this in the meeting today. @blink1073 suggested that the --alias flag might be preferable because it's easier to document in our CLI, so I think we should go with that.

However, instead of --alias another name like --pin-version or --pin-version-as to attempt to be clearer about the users intent, which is that they want an older version of a package alongside the normal version.

We also discussed how these extensions should update. One option would be to use the semantic versioning range specified by the user, for example:

jupyter labextension install --pin-version-as test-extension-1 jupyterlab-test-extension@^1.0.0 

If this installs version 1.0.0 and then a 1.0.1 gets released, then a jupyter labextension update test-extension-1 would update to that release.

My preference would be to not have any update semantics for pinned versions. If you try to update one it will just fail and if you update all, then it will ignore these installs.

@saulshanabrook saulshanabrook added this to the 1.1 milestone Jul 24, 2019
@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 25, 2019

I have updated the PR description to use the command line flag.

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 25, 2019

Ah darn, I realized a reason to go back to using the yarn alias syntax instead... Multiple package installs. For example, this doesn't make any sense jupyter labextension install --pin-version-as test-1 jupyterlab-test-extension@1.0 jupyterlab-test-extension@2.0 whereas jupyter labextension install test1@npm:jupyterlab-test-extension@1.0 test2@npm:jupyterlab-test-extension@2.0 does.

It seems kinda ugly to not support multiple arguments only when the pin flag is present.

I guess alternatively, the pin flag could take multiple comma separated names? So that it would be jupyter labextension install --pin-version-as test-1,test-2 jupyterlab-test-extension@1.0 jupyterlab-test-extension@2.0

@saulshanabrook saulshanabrook marked this pull request as ready for review Jul 25, 2019
@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 25, 2019

OK this is good to go from my perspective. It should match the syntax laid out in the PR description.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Jul 29, 2019

I like it overall, thanks! Do you mind putting some of the PR description language in the docstring for InstallLabExtensionApp along the lines of https://github.com/jupyter/notebook/blob/e498de6775b3de57f5ff827e562c179b17893064/notebook/nbextensions.py#L645

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Jul 31, 2019

Do you mind putting some of the PR description language in the docstring for InstallLabExtensionApp

Sure, I had a first pass at this.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 1, 2019

Thanks @saulshanabrook, the language looks good. This needs a rebase though.

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Aug 1, 2019

@blink1073 Just merged in!

Copy link
Member

@blink1073 blink1073 left a comment

👍🏼

Copy link
Member

@vidartf vidartf left a comment

Mostly looks good. One comment about readability, and one smaller issue.

jupyterlab/commands.py Show resolved Hide resolved
jupyterlab/commands.py Outdated Show resolved Hide resolved
@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Aug 2, 2019

I also just added a test to CI to install two versions of the same extension and then uninstall them. Ideally, it would be nice to verify they actually both load by opening the browser and checking the console logs. However, we don't have the ability to do this yet in our tests.

scripts/travis_script.sh Outdated Show resolved Hide resolved
Otherwise I get:

```
E       AssertionError: '/var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/tmpfn25l4mq/data/lab' != '/private/var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/tmpfn25l4mq/data/lab'
E       - /var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/tmpfn25l4mq/data/lab
E       + /private/var/folders/m7/t8dvwtnn32z84333p845tly40000gn/T/tmpfn25l4mq/data/lab
E       ? ++++++++
```
@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Aug 7, 2019

I need to fix the behavior on windows, the tests seem to be failing there.

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Aug 8, 2019

Does anyone who runs on windows have any ideas about why these tests are failing there? The extension doesn't seem to be installed properly. I assume it is because some system command I am running is behaving differently on windows than on mac/linux.

@vidartf
Copy link
Member

@vidartf vidartf commented Aug 10, 2019

Haven't checked in detail, but typically a shell=os.name=='nt' (or however we check for windows in lab code) helps.

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Aug 15, 2019

Ah, the issue was I was putting a : in the path name and this isn't allowed in windows! I will switch it to a @

@saulshanabrook
Copy link
Member Author

@saulshanabrook saulshanabrook commented Aug 15, 2019

I believe the test failure is unrelated. This is ready to go.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 15, 2019

Thanks!

@blink1073 blink1073 merged commit 0acc990 into master Aug 15, 2019
7 of 9 checks passed
@vidartf vidartf deleted the extension-alias branch Aug 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants