-
Notifications
You must be signed in to change notification settings - Fork 94
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
Pre package extensions and install them on startup for users #6403
Conversation
E2E Tests 🚀 |
src/vs/platform/extensionManagement/node/positronBootstrapExtensionsInitializer.ts
Show resolved
Hide resolved
d092f62
to
2b30e74
Compare
42b71a0
to
0ca3194
Compare
@melissa-barca I updated so the web based tests will run on the PR as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a release build and a dev build and the extension experience is looking great!
For the failing test, I suspect what is happening is that our positron-ipywidgets
extension, which is truly builtin
, must somehow need one of the jupyter extensions to work and something is going wrong there. Here is our builtin
extension:
https://github.com/posit-dev/positron/tree/main/extensions/positron-ipywidgets
@seeM I know you worked on this; do you have any thoughts on how our extension here uses other extensions, that we're trying not have builtin
? I see there are no official dependencies for it.
Thanks @juliasilge! I suspected something like that with the test, because it started failing when I moved the Jupyter tests over but like you mentioned, I didn't see a declared dependency. I'm wondering if I should move them (or isolate the problematic one and just move that one) back to built-in and file a separate issue for that. |
If it is the main Jupyter extension, I would say that is one of the driving examples of us wanting this bundling type of experience; it would be great to resolve the failing test without moving it back to |
defaultExtensionsInitializer so it can be applied in server and desktop
3d443ad
to
0b1ec40
Compare
0b1ec40
to
a15e0a1
Compare
@juliasilge I did some local testing and the tests pass when I move jupyter-renderers back to built-in. How do you feel about keeping that one? |
In the immediate short term, let's go for it! 🚀 For the slightly longer term, can you open an issue outlining why we couldn't bootstrap this one and move it to triage? We'll get it prioritized for a quick followup, to find out how to address the problem. |
Follow up issue for the Jupyter Notebook Renderers extension is here: #6507 |
Addresses #6302, #6301, #6305, #5315
Introduces the concept of "bootstrapped extensions". Bundles the following extensions as VSIX files with Positron:
When a user starts up Positron after first installing it or a version change (upgrade or downgrade), Positron will install the bootstrapped extensions for the users. Users will then be able to manage the extensions (upgrade, downgrade, uninstall, disable) themselves. Positron will always prefer whichever version of the extension in newer (bundled or user). When a user downgrades the extension, Positron will attempt to upgrade to the latest packaged version on the next upgrade, but not next Positron launch.
I changed the approach slightly from my initial spec. The initial spec built on top of the existing defaultExtensionInitializer. This approach is heavily inspired by that class but provides the server and desktop logic in src/vs/platform/extensionManagement/node/positronBootstrapExtensionsInitializer.ts.
Release Notes
New Features
Bug Fixes
QA Notes
@:web