takluyver / jupyter_kernel_mgmt

Experimental new kernel management framework for Jupyter
https://jupyter-kernel-mgmt.readthedocs.io/en/latest/
Other
15 stars 8 forks source link

Add support for kernel launch parameters #22

Closed kevin-bates closed 5 years ago

kevin-bates commented 5 years ago

These changes add the ability for kernel provider clients to pass in "launch" parameters. I chose "launch parameters" over "kernel parameters" because many of the parameters that a client will provide will be used to seed the environment in which the kernel will run, and not necessarily be conveyed to the kernel directly.

More importantly than the launch_params argument added to the launch() methods, the test code illustrates how the metadata corresponding to the available parameters could be implemented. In this case, the provider is a Spec provider, so the metadata is pulled directly from the kernelspec. Other providers may choose to do things differently, but the end result is that the metadata entry returned from find_kernels() would include a launch_params (or whatever we call it) stanza consisting of a list of dicts where each element is a parameter and its dict describes that parameter.

\

Here's the example spec used in the test code...

"metadata": {
  "launch_parameter_schema": {
    "title": "Params_kspec Kernel Provider Launch Parameter Schema",
      "properties": {
        "line_count": {"type": "integer", "minimum": 1, "default": 20, "description": "The number of lines totail"}, 
        "follow": {"type": "string", "enum": ["-f", "-F"], "default": "-f", "description": "The follow option to tail"}, 
        "cpus": {"type": "number", "minimum": 0.5, "maximum": 8.0, "default": 4.0, "description": "The number of CPUs to use for this kernel"},
        "memory": {"type": "integer", "minimum": 2, "maximum": 1024, "default": 8, "description": "The number of GB to reserve for memory for this kernel"}
      },
      "required": ["line_count", "follow"]
  } 
}

The client (front-end) would then use the metadata to prompt for values and create a simple dictionary of name, value pairs. These then get re-validated by the provider against the metadata. Required values that were not provided but have specified defaults are set, ranges are checked (as appropriate), etc. Once validated, the parameters are used in whichever way the provider has defined.

Copying a few other folks that might be interested: @SylvainCorlay @zsailer @lresende @rolweber @jasongrout @blink1073

EDIT: replaced previously custom schema with JSON schema referenced in comment below (and added Steve to cc list).

takluyver commented 5 years ago

We may instead want a dict of dicts where the key is the parameter's symbolic_name

I kind of like the list-of-dicts design. Not a strong preference, but if you're going to generate documentation or an interactive form from it, you want to write an ordered collection. I know recent Python will probably preserve the order, but JSON objects are defined as unordered, and you may want to reserialise it and send it to some other code.

kevin-bates commented 5 years ago

Good point about ordered dicts. The reason I prefer metadata be a dict of dicts is for correlation in the launch methods of the provider. In order to validate parameters, the client-provided parameter should be easily found in the corresponding metadata. By using the parameter name as the key to its metadata, this is quick and easy; much more so that scanning each element asking if it's name has a value of <parameter>. (I'm sure there's some python trick to perform that in an elegant manner, but I can't see it beating a dictionary lookup.)

codecov-io commented 5 years ago

Codecov Report

Merging #22 into master will increase coverage by 0.11%. The diff coverage is 98.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   70.95%   71.07%   +0.11%     
==========================================
  Files          27       27              
  Lines        2045     2043       -2     
==========================================
+ Hits         1451     1452       +1     
+ Misses        594      591       -3
Impacted Files Coverage Δ
jupyter_kernel_mgmt/tests/test_discovery.py 96.58% <100%> (+0.89%) :arrow_up:
jupyter_kernel_mgmt/tests/test_kernelspec.py 99.21% <100%> (ø) :arrow_up:
jupyter_kernel_mgmt/subproc/launcher.py 74.72% <100%> (+0.56%) :arrow_up:
jupyter_kernel_mgmt/discovery.py 61.38% <90.9%> (-1%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4902927...00b4950. Read the comment docs.

kevin-bates commented 5 years ago

After experimenting with JSON schema, I think the best approach would be for the launch parameters to be defined in native JSON schema. (Originally, I was thinking that we would build a schema so we could entertain custom properties, but I think pure JSON schema is more familiar and tools would adhere to it better. In addition, parameter validation would be more straight forward.

I've updated the test to use this to define the launch_parameters:

"metadata": {
  "launch_parameter_schema": {
    "title": "Params_kspec Kernel Provider Launch Parameter Schema",
      "properties": {
        "line_count": {"type": "integer", "minimum": 1, "default": 20, "description": "The number of lines totail"}, 
        "follow": {"type": "string", "enum": ["-f", "-F"], "default": "-f", "description": "The follow option to tail"}, 
        "cpus": {"type": "number", "minimum": 0.5, "maximum": 8.0, "default": 4.0, "description": "The number of CPUs to use for this kernel"},
        "memory": {"type": "integer", "minimum": 2, "maximum": 1024, "default": 8, "description": "The number of GB to reserve for memory for this kernel"}
      },
      "required": ["line_count", "follow"]
  } 
}
kevin-bates commented 5 years ago

Just remembered that Kyle might be interested in this as well: @rgbkrk

Zsailer commented 5 years ago

After experimenting with JSON schema, I think the best approach would be for the launch parameters to be defined in native JSON schema

:+1: for defining in native JSON Schema.

You probably haven't been following the Jupyter Telemetry or Metadata work, but defining JSON schemas is foundational to this work. It would be awesome to hook the kernel provider work into Telemetry and Metadata systems. We could easily emit/store and validate kernel provider events using your schema.

takluyver commented 5 years ago

Feel free to resolve the conflicts and merge this.