-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
Custom python environment support #317
Conversation
6cffc8b
to
c081800
Compare
@@ -258,6 +259,7 @@ | |||
"@types/semver": "^7.3.4", | |||
"@types/yargs": "^16.0.0", | |||
"bottlejs": "^2.0.0", | |||
"ejs": "^3.1.6", |
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 was thinking more renderToString
style from react-dom
(which already is a dependency), but I guess that would work too. I don't think yarn.lock
was updated, how much does inclusion of ejs
costs us in terms of extra dependencies?
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.
ejs was already included through another dependency. I will also check out react-dom renderToString you mentioned.
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.
@krassowski I looked into ReactDOMServer.renderToString. it doesn't look like an easy integration for our use case. we generate the HTML of the dialogs on the app backend using node.js. Dialog front-ends are quite basic HTML and JS. We would need to use react for dialog front-ends which would be significant overhead just for sanitize purposes. Also, sanitization would happen on the front-end which may not be as secure as intended.
Please let me know if I missed something and if you see an easier integration of ReactDOMServer.renderToString.
Hm, it looks like my reply was swallowed by the aether (happens), here's a paraphrase: Strictly from a Some approaches:
At any rate, perhaps an appropriate play is to handle the two incumbent providers explicitly, implemented as the former, and then a Customize Template... which would allow someone to further tweak them, or replace entirely with e.g. |
@bollwyvl thanks for bringing this up. conda run looks pretty nice but venv requires a separate solution too. We should go for creating launch scripts that take care of env activation and then launching jlab as you suggested. As far as I could see, our packages in the bundled environment don't have pre-activate/activate scripts and this case seems to be rare. So, I think we can implement launch script generation in another PR. |
9e90f13
to
5f6dfbc
Compare
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, thank you!
Adding basic support for custom Python environment installed on user machine
conda: env_name
(for conda),venv: env_name
(for virtualenv),system: python
(for other cases)Status bar item
Changing environment
Failed launch due to incompatible environment