-
Notifications
You must be signed in to change notification settings - Fork 144
[WIP] Add ConfigMap builder #21
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import os | ||
import subprocess | ||
import tempfile | ||
import shutil | ||
|
||
from kubernetes import client | ||
|
||
from fairing import notebook_helper | ||
from .builder import BuilderInterface | ||
from fairing.backend import kubernetes | ||
|
||
|
||
class ConfigMapBuilder(BuilderInterface): | ||
|
||
def __init__(self, notebook_name=None): | ||
if notebook_name is None: | ||
self.notebook_name = notebook_helper.get_notebook_name() | ||
else: | ||
self.notebook_name = notebook_name | ||
|
||
def execute(self, namespace, job_id, base_image): | ||
nb_full_path = os.path.join(os.getcwd(), self.notebook_name) | ||
temp_dir = tempfile.mkdtemp() | ||
code_path = os.path.join(temp_dir, 'code.py') | ||
try: | ||
cmd = "jupyter nbconvert --to python {nb_path} --output {output}" | ||
.format( | ||
nb_path=nb_full_path, | ||
output=code_path | ||
).split() | ||
|
||
subprocess.check_call(cmd) | ||
with open(code_path, 'rb') as f: | ||
code = f.read() | ||
finally: | ||
shutil.rmtree(temp_dir, ignore_errors=True) | ||
|
||
client.V1ConfigMap( | ||
api_version="v1", | ||
data={"code.py": code}, | ||
metadata=client.V1ObjectMeta(name=self.job_id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is job_id a parameter or member? Who sets it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
) | ||
k8s = kubernetes.KubeManager() | ||
k8s.create_config_map(namespace, config_map) | ||
return self._generate_pod_spec(job_id, base_image) | ||
|
||
def _generate_pod_spec(self, job_id, base_image): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if it would be this PR but would be great to provide this pod-spec using a configmap(that way specifying images, environments, and other mounts is completely upto the user). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be part of the discussion here: #28 and be in it's own PR. |
||
volume_name = 'code' | ||
return client.V1PodSpec( | ||
containers=[client.V1Container( | ||
name='model', | ||
image=base_image, | ||
command="python /code/code.py".split(), | ||
volume_mounts=[client.V1VolumeMount(name=volume_name, mount_path='/code')] | ||
)], | ||
restart_policy='Never', | ||
volumes=client.V1Volume( | ||
name=volume_name, | ||
config_map=client.V1ConfigMapVolumeSource(name=job_id) | ||
) | ||
) |
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.
Does this work? If not, we should make notebook_name required.
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.
It works in standalone notebooks, not with JupyterHub.
I can improve the error handling in the helper to display an explicit error when user JH such as: "notebook_name is required when using JupyterHub"