Jupyter Interactive Tool 1.0.1/24.07

[natefoo]

Note: You need to set $NB_UID in your job conf environment (destination) for this tool.

docker_run_extra_arguments: "-e NB_UID=$(id -u)"

I added <required_files> for running via Pulsar but this is otherwise just a version update from tools/interactive/interactivetool_jupyter_notebook_1.0.0.xml.

How to test the changes?

• [x] This is a refactoring of components with existing test coverage.

License

• [x] I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[natefoo]

I just realized maybe we prefer to update the 1.0.0 tool instead since it is functionally the same (and the functional difference is why I assume 0.3 and 1.0.0 were made separate in the first place).

[mvdbeek]

Note: You need to set $NB_UID in your job conf environment (destination) for this tool.

Why is that, and can we avoid this ?

[natefoo]

@bgruening correct me if I'm wrong, but the upstream Jupyter/Julia image on which it's based starts as root and drops privileges to $NB_UID. If unset then jupyter runs as jovyan and can't write to the working dir. To change this would require building fully custom images.

[mvdbeek]

That seems like a much better idea to me, if NB_UID can't be set to 0.

[natefoo]

Building fully custom images? That is a lot of work to keep updated since we can't just build off the already maintained upstream images.

[mvdbeek]

Yeah, but you can't write in your PR "oh and btw put this in your job conf" and expect it'll be done without trial and error. So now we could build out an abstraction for this ... or we do the "container thing" of not dropping privileges. Surely you can overwrite that one script that drops privileges ?

[mvdbeek]

(I'd also check if NB_UID=0 works)

[natefoo]

That might work depending on the deployment, but not for root-squashed filesystems, and would make cleanup more difficult.

[mvdbeek]

I admit I'm not an admin, but aren't you always 0 in a standard container and your container engine manages the id translation ?

❯ docker run quay.io/biocontainers/galaxy-data:24.1.1--pyhdfd78af_0 id
uid=0(root) gid=0(root) groups=0(root)

Are you saying the problem is not that this joyvan image is trying to be a small VM when it shouldn't ?

[natefoo]

You are root unless you 1. pass --user or 2. the container's CMD or ENTRYPOINT do something to change users, which is not uncommon among containers that run fairly complex applications.

[mvdbeek]

You are root unless you 1. pass --user or 2. the container's CMD or ENTRYPOINT do something to change users, which is not uncommon among containers that run fairly complex applications.

That is a good point, however then I would set $NB_UID=$(id) in the container

he container's CMD or ENTRYPOINT do something to change users,

but I'm suggesting to just not do that, which seems feasible for this case ? Let's avoid complexity where we can, and otherwise I think we should at least have language to say "This tool requires the following environment variable to be set"

[natefoo]

That is a good point, however then I would set $NB_UID=$(id) in the container

afaik we can't without modifying the upstream container.

but I'm suggesting to just not do that, which seems feasible for this case ? Let's avoid complexity where we can, and otherwise I think we should at least have language to say "This tool requires the following environment variable to be set"

I presume there is a reason why they do this, people more familiar with the upstream image probably know why. I am happy to document it but we still don't really have a way for tool authors to provide documentation to admins, do we?

[natefoo]

@mvdbeek is there something you'd like me to do here? FWIW this is not a change from the behavior of the existing 1.x version of this tool.

[github-actions[bot]]

This PR was merged without a "kind/" label, please correct.

[mvdbeek]

Ran into this during the admin training 🤣

[natefoo]

Definitely something we need to deal with more properly, do we have an issue?