Skip to content

Commit

Permalink
Fix spawner_ui_config.yaml to use correct null values for configurati…
Browse files Browse the repository at this point in the history
…ons (#361)

Previously, the spawner_ui_config.yaml was rendered with empty strings and lists being rendered as null values in the configuration file.  For example if the GPU vendors list was empty and the default vendor was `""`,  the config file would have (shown truncated):

```
  gpus:
    value:
      vendor:

      vendors:
```

whereas jupyter web app expected:
```
  gpus:
    value:
      vendor: ""

      vendors: []
```

This commit updates the template to ensure we always output the correct empty values.

Closes #360
  • Loading branch information
ca-scribner authored Apr 29, 2024
1 parent 0d1bbcf commit 211a37b
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 27 deletions.
52 changes: 45 additions & 7 deletions charms/jupyter-ui/src/templates/spawner_ui_config.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,18 @@ spawnerFormDefaults:
################################################################
image:
# the default container image
{%- if jupyter_images_default %}
value: {{ jupyter_images_default }}
{%- else %}
value: ""
{%- endif %}

# the list of available container images in the dropdown
options:
{%- if jupyter_images | length > 0 %}
options:
{{ jupyter_images | to_yaml | indent(4) }}
{%- else %}
options: []
{%- endif %}

################################################################
Expand All @@ -59,12 +65,18 @@ spawnerFormDefaults:
################################################################
imageGroupOne:
# the default container image
{%- if vscode_images_default %}
value: {{ vscode_images_default }}
{%- else %}
value: ""
{%- endif %}

# the list of available container images in the dropdown
options:
{%- if vscode_images | length > 0 %}
options:
{{ vscode_images | to_yaml | indent(4) }}
{%- else %}
options: []
{%- endif %}

################################################################
Expand All @@ -82,12 +94,18 @@ spawnerFormDefaults:
################################################################
imageGroupTwo:
# the default container image
{%- if rstudio_images_default %}
value: {{ rstudio_images_default }}
{%- else %}
value: ""
{%- endif %}

# the list of available container images in the dropdown
options:
{%- if rstudio_images | length > 0 %}
options:
{{ rstudio_images | to_yaml | indent(4) }}
{%- else %}
options: []
{%- endif %}

################################################################
Expand Down Expand Up @@ -127,14 +145,20 @@ spawnerFormDefaults:
value:
# the `limitKey` of the default vendor
# (to have no default, set as "")
{%- if gpu_vendors_default %}
vendor: {{ gpu_vendors_default }}
{%- else %}
vendor: ""
{%- endif %}

# the list of available vendors in the dropdown
# `limitsKey` - what will be set as the actual limit
# `uiName` - what will be displayed in the dropdown UI
vendors:
{%- if gpu_vendors | length > 0 %}
vendors:
{{ gpu_vendors | to_yaml | indent(6) }}
{%- else %}
vendors: []
{%- endif %}

# the default value of the limit
Expand Down Expand Up @@ -200,12 +224,18 @@ spawnerFormDefaults:
# the `configKey` of the default affinity config
# (to have no default, set as "")
# (if `readOnly`, the default `value` will be the only accessible option)
{%- if affinity_options_default %}
value: {{ affinity_options_default }}
{%- else %}
value: ""
{%- endif %}

# the list of available affinity configs in the dropdown
options:
{%- if affinity_options | length > 0 %}
options:
{{ affinity_options | to_yaml | indent(4) }}
{%- else %}
options: []
{%- endif %}

#options:
Expand Down Expand Up @@ -243,12 +273,18 @@ spawnerFormDefaults:
# the `groupKey` of the default toleration group
# (to have no default, set as "")
# (if `readOnly`, the default `value` will be the only accessible option)
{%- if tolerations_options_default %}
value: {{ tolerations_options_default }}
{%- else %}
value: ""
{%- endif %}

# the list of available toleration groups in the dropdown
options:
{%- if tolerations_options | length > 0 %}
options:
{{ tolerations_options | to_yaml | indent(4) }}
{%- else %}
options: []
{%- endif %}

#options:
Expand Down Expand Up @@ -301,9 +337,11 @@ spawnerFormDefaults:

# the list of PodDefault names that are selected by default
# (take care to ensure these PodDefaults exist in Profile Namespaces)
value:
{%- if default_poddefaults | length > 0 %}
value:
{{ default_poddefaults | to_yaml | indent(4) }}
{%- else %}
value: []
{%- endif %}

################################################################
Expand Down
31 changes: 11 additions & 20 deletions charms/jupyter-ui/tests/unit/test_operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#

"""Unit tests for JupyterUI Charm."""

import copy
import logging
from contextlib import nullcontext as does_not_raise
from unittest.mock import MagicMock, patch
Expand Down Expand Up @@ -393,21 +393,23 @@ def test_notebook_configurations(
(
dict(
jupyter_images_config=OptionsWithDefault(
default="", options=["jupyterimage1", "jupyterimage2"]
default="jupyterimage1", options=["jupyterimage1", "jupyterimage2"]
),
vscode_images_config=OptionsWithDefault(
default="", options=["vscodeimage1", "vscodeimage2"]
default="vscodeimage1", options=["vscodeimage1", "vscodeimage2"]
),
rstudio_images_config=OptionsWithDefault(
default="", options=["rstudioimage1", "rstudioimage2"]
default="rstudioimage1", options=["rstudioimage1", "rstudioimage2"]
),
gpu_number_default=1,
gpu_vendors_config=OptionsWithDefault(default="", options=GPU_VENDORS_CONFIG),
gpu_vendors_config=OptionsWithDefault(
default="nvidia", options=GPU_VENDORS_CONFIG
),
affinity_options_config=OptionsWithDefault(
default="", options=AFFINITY_OPTIONS_CONFIG
default="test-affinity-config-1", options=AFFINITY_OPTIONS_CONFIG
),
tolerations_options_config=OptionsWithDefault(
default="", options=TOLERATIONS_OPTIONS_CONFIG
default="test-tolerations-group-1", options=TOLERATIONS_OPTIONS_CONFIG
),
default_poddefaults_config=OptionsWithDefault(
default="", options=DEFAULT_PODDEFAULTS_CONFIG
Expand All @@ -428,19 +430,8 @@ def test_render_jwa_file(
# Arrange
render_args = render_jwa_file_with_images_config_args

# Build the expected results, converting empty values to None to match the output of the
# function
expected = {
k: (
OptionsWithDefault(
default=(config.default if config.default else None),
options=(config.options if config.options else None),
)
if k != "gpu_number_default"
else config
)
for k, config in render_args.items()
}
# Build the expected results
expected = copy.deepcopy(render_args)

harness.set_leader(True)
harness.begin()
Expand Down

0 comments on commit 211a37b

Please sign in to comment.