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.

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

Consider alternative docker image atttribute as kernel_id #19

Open
Lubricy opened this issue Apr 4, 2021 · 2 comments
Open

Consider alternative docker image atttribute as kernel_id #19

Lubricy opened this issue Apr 4, 2021 · 2 comments
Labels
🐛 bug Something isn't working ⚙ config Issues and requests related to user configuration ✨ enhancement New feature or request 🔜 high priority If you're looking for something to work on - consider this first

Comments

@Lubricy
Copy link

Lubricy commented Apr 4, 2021

First of all great work of yours! appreciate it.

Though I met some problems when trying to install a docker image as a KernelSpec (tensorflow/tensorflow:latest-gpu to be precise).
got an Error

ValueError: kernelspec already exists: /home/lubricy/.local/share/jupyter/kernels

it turns out that image.attrs['ContainerConfig']['Hostname'] is just an empty string.

I'm not quite familiar with docker image attrs but perhaps a sanitized image tag (e.g. tensorflow_tensorflow_latest-gpu)would work better here? or perhaps rather let user to decide the name/id of there kernel?

Another unrelated thing is I think it would be nice if dockernel start (and dockernel install) could accept docker cli flags (e.g. -v VOLUME , -p PORT, -u USER etc.). a particular flag I'd like to use is --gpus all but it seems like it's not natively supported by py-docker upstream. Maybe give python-on-whales a try?

@MrMino
Copy link
Owner

MrMino commented Apr 4, 2021

Hi @Lubricy! Thank you, I'm glad you like it!

I had no idea that ['Hostname'] key could point to an empty string there. At the very least that's something that needs to be shielded against.

Looking back at it, I'm not sure whether it was a good idea to use Hostname there at all. IIRC this was just a temporary bodge, so that I don't have to look after something more proper to put there. I'll surely need to add an --id argument into dockernel install so that even if it bites, its workaround-able.

I plan on putting some of the container configuration into a per-user setting, so that e.g. you can specify the volumes you work on inside a ~/.dockernel config file and use them for every kernel. That topic is still very much open, so if you have any suggestions - I'd love to hear them.

With regards to --gpus all - I'll have to look into it. Are you sure it's not supported? Looks to me like it was merged last year via docker/docker-py#2471, I'll have to check.

Note to self:

@MrMino MrMino added 🐛 bug Something isn't working ✨ enhancement New feature or request ⚙ config Issues and requests related to user configuration 🔜 high priority If you're looking for something to work on - consider this first labels Apr 4, 2021
@Lubricy
Copy link
Author

Lubricy commented Apr 10, 2021

took a second look on the PR - yeah you are right it's supported. I got confused by those still-open issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working ⚙ config Issues and requests related to user configuration ✨ enhancement New feature or request 🔜 high priority If you're looking for something to work on - consider this first
Projects
None yet
Development

No branches or pull requests

2 participants