-
Notifications
You must be signed in to change notification settings - Fork 18
SwanProjects extension for jupyter lab 3 #179
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
Conversation
Thanks a lot @etejedor for the review |
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.
Almost ready to merge!!
But please let's remove the necessity to be inside SWAN_projects as soon as possible.
# | ||
# * HOME with path to the user home | ||
# * PATH with default paths for the system | ||
# * OAUTH2_TOKEN required by EOS storage |
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.
Do we really need OAUTH2_TOKEN
otherwise the EOS client doesn't work?
But if that is the case, this seems hardcoded to me...
We do not inherit all variables to avoid polluting the env, right? But can't we make it configurable outside of this extension to allow setting some extra env vars in all projects?
This solution is not great and I hope that we can do it differently once we stop configuring the lcg stack in the startup. So at least we should put a "fixme" here if that is the case.
proc = subprocess.Popen(command, stdout=subprocess.PIPE) | ||
proc.wait() | ||
proc.communicate() | ||
return proc.returncode |
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.
return proc.returncode == 0
and then you don't need to the if..else
later on,
}; | ||
return ( | ||
<div | ||
// this style can not be moved to the css file, |
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.
You can still put it in the css file and use props.isSelected
to decide wether to add an extra '.selected' class or not.
}; | ||
return ( | ||
<div | ||
// this style can not be moved to the css file, |
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.
You can still put it in the css file and use props.isSelected
to decide wether to add an extra '.selected' class or not.
if (old_options.name != options.name) { | ||
await commands | ||
.execute('filebrowser:go-to-path', { | ||
path: '/SWAN_projects', |
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.
Assuming that it exists in SWAN_projects
, which might not be the case. Can't you go to the parent folder instead?
* @param init Initial values for the request | ||
* @returns The response body interpreted as JSON | ||
*/ | ||
export async function request<T>( |
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'm having a dejavu. This same function is used in SwanFileBrowser
, right? Can we improve this? Can we use that extension from jupyterlab-contrib
?
options: ProjectDialog.ISWANOptions | ||
): any { | ||
const dataToSend = { | ||
old_name: old_options.name, |
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.
renameKeys = (obj) =>
Object.keys(obj).reduce(
(acc, key) => ({
...acc,
...{ [`old_${key}`]: obj[key] }
}),
{}
);
Inspiration from https://www.freecodecamp.org/news/30-seconds-of-code-rename-many-object-keys-in-javascript-268f279c7bfa/.
then you can do ...renameKeys(old_options),
c.NotebookApp.default_url = 'lab' | ||
c.NotebookApp.contents_manager_class = 'swancontents.filemanager.swanfilemanager.SwanFileManager' | ||
c.NotebookApp.kernel_spec_manager_class = 'swanprojects.kernelmanager.kernelspecmanager.SwanKernelSpecManager' | ||
c.KernelSpecManager.ensure_native_kernel = False |
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.
Add a comment explaining
c.NotebookApp.kernel_spec_manager_class = 'swanprojects.kernelmanager.kernelspecmanager.SwanKernelSpecManager' | ||
c.KernelSpecManager.ensure_native_kernel = False | ||
|
||
c.SwanConfig.stacks_path=path_to_stacks_folder |
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.
Add comment saying this is optional or explaining the default values.
And should be c.SwanConfig.stacks_path = '/my/path'
@@ -0,0 +1,36 @@ | |||
{ | |||
"releases":{ | |||
"LCG_100": [ |
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.
Should we have better naming? Like what we do in production. And then we could divide into categories like we do now as well.
Closing as it was superseded by #209 |
Hi Guys,
this is the SwanProjects Jupyter Lab extension with backend and dialogs to Create/Edit projects, also have a customized KerneSpecManager to handle kernels in multiple environments.
Tested on swan-spare001 and with jupyterlab version 3.0.x
Cheers,
Omar.