[1464] Move z2jh.py to python and distribution agnostic path

[mrow4a]

Path /usr/local/lib/python3.6/dist-packages binds chart to Ubuntu-like distribution and Python 3.6 docker images.

Currently, configuration script is placed in /etc/jupyterhub/jupyterhub_config.py and JupyterHub execs it.

jupyterhub
--config /etc/jupyterhub/jupyterhub_config.py
...

Thus, it is not enough that z2jh.py is in /etc/jupyterhub/ as it is not from where the python config is executed. Thus, the optimal solution could be to add the script to PYTHONPATH to container spec.

Closes #1464
@consideRatio @mkjpryor-stfc

[manics]

This potentially overrides a PYTHONPATH set by the user.

Since this is mostly internal to the image how about insert /etc/jupyterhub/ to sys.path, e.g. in jupyterhub_config.py

import sys
sys.path.insert(0, '/etc/jupyterhub')
from z2jh import get_config, set_config_if_not_none

(I haven't tested this)

[mrow4a]

@manics how would you define PYTHONPATH for the container before env is set for the container?

If you set PYTHONPATH before container starts (as part of container ENV definition like in this PR), all other operations in entrypoint (where python is loaded too) will just append to that PYTHONPATH. If you have some custom PYTHONPATH, you probably would do it as part of entrypoint (which will append to PYTHONPATH), but firstly defined is PYTHONPATH container env. Am I right?

[manics]

It could be set with hub.extraEnv:

zero-to-jupyterhub-k8s/jupyterhub/templates/hub/deployment.yaml

Line 181 in 5eb48bc

{{- if .Values.hub.extraEnv }}

[manics]

Expanding on my rationale: I think of PYTHONPATH as a last resort for users when there's no other way of configuring Python other than completely rebuilding the image, so where possible I think we should keep this option open to them (in all projects, not just this one).

[consideRatio]

Hmmm, I consider this PR very relevant, so it needs to be fixed somehow. Either by PYTHONPATH, or by modifying the sys.path etc... I semi-verified that it would work to use @manics suggestion in #1478 (comment).

The JupyterHub binary should be allowed to be anywhere when it is started.
The configuration file should preferably not assume stuff, but it is quite nice that we would ask for it to use specifically /etc/jupyterhub as that kinda make some sense to load stuff from, of all places... I don't know for sure what I think is best.
The helm chart is already mounting the z2jh, so it makes sense that it would also manage information about how to access it through a change to PYTHONPATH for example.

I'm open to both using PYTHONPATH and sys.path.insert. As I don't feel strongly I prefer to sidestep on that discussion.

[mrow4a]

@consideRatio @manics I took an approach of sys.insert. To me it looks clean - all modules belonging to the configuration script should be in the directory where configuration script is.

[manics]

I like it!

One thing I'm not sure about, @consideRatio may know better: k8s may symlink config files into place, is there any situation where the real parent directories of this file and z2jh.py would be different?

[consideRatio]

I don't know, I think though that if this works for the TravisCI test pipeline, than I think it is robust enough as it worked in that k8s setup at least.

[mrow4a]

@manics if you mount it like

- mountPath: /etc/jupyterhub/z2jh.py
   subPath: z2jh.py
   name: config

the file is regular file, not a symlink. Symlink happens only in case of mounting secrets as a directory, which in z2jh chart is not possible to do into /etc/jupyterhub.

- mountPath: /etc/jupyterhub/config/
  name: config

[mrow4a]

Anyways even if this would be moved to ConfigMap directory, the directory there is still a directory as https://docs.python.org/2/library/os.path.html#os.path.realpath ignores symlinks

$ root@hub-755dbdc69c-tt7r4:/srv/jupyterhub# python -c "import os;print(os.path.realpath('/etc/jupyterhub/config/z2jh.py'))"
/etc/jupyterhub/config/..2019_11_13_14_20_12.271060545/z2jh.py

[consideRatio]

This LGTM! Want to merge @manics ? :)

Thank you @mrow4a for your work to report, explain, and solve this issue! 🎉 ❤️ 🌻 !

[manics]

Thanks @mrow4a!