Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable ability to not filter kernels in container deployments #1131

Merged
merged 3 commits into from
Jul 18, 2022
Merged

Enable ability to not filter kernels in container deployments #1131

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d46729
Enable ability to not filter kernels in container deployments
kevin-bates Jul 12, 2022
00838d7
Join allowed-kernels option with log-level to avoid extra escape when…
kevin-bates Jul 13, 2022
e2a0fa3
Switch default whitelist env value to null, add option only if not null
kevin-bates Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions etc/docker/enterprise-gateway/start-enterprise-gateway.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,19 @@ EG_KERNEL_WHITELIST=${EG_KERNEL_WHITELIST:-"'r_docker','python_docker','python_t
export EG_KERNEL_WHITELIST=`echo ${EG_KERNEL_WHITELIST} | sed 's/[][]//g'` # sed is used to strip off surrounding brackets as they should no longer be included.
export EG_DEFAULT_KERNEL_NAME=${EG_DEFAULT_KERNEL_NAME:-python_docker}

# Determine whether the kernels-allowed list should be added to the start command.
# This is conveyed via a 'null' value for the env - which indicates no kernel names
# were used in the helm chart.
allowed_kernels_option="--KernelSpecManager.whitelist=[${EG_KERNEL_WHITELIST}]"
if [ "${EG_KERNEL_WHITELIST}" == 'null' ]; then
allowed_kernels_option=""
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes more sense to have allowed_kernels_option default to "". Then change the if to check if "${EG_KERNEL_WHITELIST}" != 'null' and set the flag inside the if.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I also switched the default value to null since I found that the docker-compose.yaml script includes the Docker-based kernel specs.



echo "Starting Jupyter Enterprise Gateway..."

exec jupyter enterprisegateway \
--log-level=${EG_LOG_LEVEL} \
--KernelSpecManager.whitelist=[${EG_KERNEL_WHITELIST}] \
--log-level=${EG_LOG_LEVEL} ${allowed_kernels_option} \
--RemoteMappingKernelManager.cull_idle_timeout=${EG_CULL_IDLE_TIMEOUT} \
--RemoteMappingKernelManager.cull_interval=${EG_CULL_INTERVAL} \
--RemoteMappingKernelManager.cull_connected=${EG_CULL_CONNECTED} \
Expand Down
4 changes: 3 additions & 1 deletion etc/kubernetes/helm/enterprise-gateway/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ kernel:
launchTimeout: 60
# Timeout for an idle kernel before its culled in seconds. Default is 1 hour.
cullIdleTimeout: 3600
# List of kernel names that are available for use.
# List of kernel names that are available for use. To allow additional kernelspecs without
# requiring redeployment (and assuming kernelspecs are mounted or otherwise accessible
# outside the pod), comment out (or remove) the entries, leaving only `whitelist:`.
whitelist:
- r_kubernetes
- python_kubernetes
Expand Down