Fix startup.sh for Galaxy's running with UID 1450.

jmchilton commented 9 years ago

This is needed because of 75e2620 and #35 right?

bgruening commented 9 years ago

@jmchilton yes it is! I hope you did not spend to much time on this bug. We fixed it already in https://github.com/bgruening/docker-ipython-notebook/tree/15.05

How can I keep to branches in sync? master and 15.05 in this case :( Big sorry.

hexylena commented 9 years ago

Uhhh now that I've woken up and y'all merged this...this worries me, https://github.com/bgruening/docker-ipython-notebook/commit/b789503efcf4ed410a68fd7a710c947e11d1b549 too.

I'm looking at that file and maybe someone can re-assure me, but it feels like it will fail in a couple ways:

if the user's id is NOT 1450, we assume they have permissions to create groups. That's a root only permission, so I'm guessing that's going to fail in unpleasant ways if UID or GID is not 0.
both of you applied the same changes, so I'm guessing that it does something correctly, but as I see it... in the case UID=1450, GID=1000 we fail the first part of the conditional, and reach the second where we su as IPython. I don't remember su playing nicely with non-interactive sessions, and I don't remember it being available to non-root users.

@bgruening (not a list of complaints, promise, your work is all awesome, just some thoughts for us):

in the future you might've rejected this PR and just opened up a PR from 15.05 to master. By merging this, we'll get merge conflicts that'll have to be manually resolved later.
we might want to switch to http://nvie.com/img/git-model@2x.png fully, i.e. work completely on dev and merge from dev to release_15.0X whenever we need.

jmchilton commented 9 years ago

@erasche I guess this goes back to my confusion on the -u comment on the RStudio container also. The IPython container needs to run as root now (and is configured to). The $uid here isn't the current container's UID (which is always and must be 0) - it is the target UID. So I think it is safe to assume it can create groups.

hexylena commented 9 years ago

Aha. That makes a lot more sense. Sorry, early morning brain for getting to me.

Since IPython now requires -u, then it's not a problem that you remove the RStudio snippet which automatically removed it in the command. That should be perfectly fine. No extra docs needed.

bgruening commented 9 years ago

@erasche ok let's develop only on :dev from now on. Are your other questions resolved with John's answer?

hexylena commented 9 years ago

Yep! We're all good! :)

bgruening / docker-ipython-notebook

Fix startup.sh for Galaxy's running with UID 1450. #43