ideonate / cdsdashboards

JupyterHub extension for ContainDS Dashboards
https://cdsdashboards.readthedocs.io/
Other
200 stars 38 forks source link

Make Dashboard Instance Size Configurable #58

Open Adam-D-Lewis opened 3 years ago

Adam-D-Lewis commented 3 years ago

Is your feature request related to a problem? Please describe. We have a jupyterhub deployment using batchspawner. We'd like to be able to configure the size of the instance (memory, cpus, etc.) on which the dashboard is deployed. Currently, when launching a new user session, JupyterHub displays a Server Options page at /hub/spawn/<user> and displays the form which we've configure via c.Spawner.options_form. It would be nice if 1) the same form specified by c.Spawner.options_form was displayed on the edit-dashboard.html page or 2) have a {% block spawner_options %}{% endblock %} added near the bottom of editdashboard.html file so that we could extend editdashboard directly without needing to redefine the entire file.

In our case, the options from the form need to end up in the new_server_options variable of the processbuilder. In jupyerhub, this seems to occur by assigning a function which reutrns a dictionary to c.Spawner.options_from_form in the config. It would be nice if setting c.Spawner.options_from_form also added the options to new_server_options of Process Builder, though there might be use cases where this isn't desirable.

Describe alternatives you've considered Currently, I got around this by overwriting editdashboard.html, subclassing BasicDashboardEditHandler, and overwriting the post method in order to add the additional form options to dashboard.options. I then subclassed ProcessBuilder and overwrote prespawn_server_options to get the form values into new_server_options defined in the ProcessBuilder.run method. Here is a link to the PR.

danlester commented 3 years ago

Adam, thank you so much for these notes. The goal makes a lot of sense.

Where you know the contents of the form in advance, then it could be easiest to do what you did and insert the form in the dashboard edit page and store the values for spawn time.

However, in the general case, obtaining the form needs a spawner to be instantiated first, and the contents of the form (or even if there is a form at all) may depend on the state of the spawner, e.g. which user it belongs to and whether they are an admin.

It feels a little bit weird to instantiate a spawner only to obtain that form, and before the spawner is really defined (we don't even know the dashboard name yet), even if it is likely to be the same form that we would get at spawn time.

I think it might be a bit more in line with the Spawner's API if there is a way to move this into the ContainDS Dashboards builder class (which is where the spawner is started if it doesn't already exist). So the form would be shown somehow after the dashboard values or saved, but before the spawner is started for the first time.

This leads to questions such as:

All of this can be decided or configurable of course - just thinking through.

I guess the main question for you is if you see a fundamental problem with the spawner form appearing at a different stage to the dashboard edit page.

Adam-D-Lewis commented 3 years ago

Thanks for your reply @danlester. For our use-case, there is no reason the spawner form has to appear on the dashboard edit page. The alternatives you propose should would work as well.

Tagging @costrouc, in case he has thoughts on this.

MarcSkovMadsen commented 3 years ago

Listening in. Being able to set cpu and memory resources and/ or the k8s nodepool to deploy to would be really helpful.