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

Make Python3.7 the default #433

Merged
merged 10 commits into from Jan 31, 2020
Merged

Conversation

GeorgianaElena
Copy link
Member

@GeorgianaElena GeorgianaElena commented Sep 18, 2019

This PR follows the steps in #250 (comment) to use Python3.7 with fresh TLJH installs, but doesn't upgrade the Python version for existing installs.

@GeorgianaElena GeorgianaElena changed the title [WIP] Make Python3.7 the default Make Python3.7 the default Sep 23, 2019
@betatim
Copy link
Member

betatim commented Jan 21, 2020

Reading this PR I realised there are two Python versions related to TLJH. The Python version in the conda environment used by users and the Python version installed on the host.

The version on the host is what is used to install and run the hub itself. The version in the conda env is what users see (their kernels run in this env).

Then there is also the version of conda that is used.

Is that also your summary? (I am new to the internals of TLJH.)


I thought that this PR was about updating the version that users see for their kernels. Looks like you are doing all three at the same time. Being a simple minded person I would try and upgrade one of them at a time (starting with the one that is seen by users). That way the number of things that can break and reasons for it breaking are kept to a minimum. What do you think?

@GeorgianaElena
Copy link
Member Author

@betatim, you summarized the setup perfectly.


I would try and upgrade one of them at a time (starting with the one that is seen by users). That way the number of things that can break and reasons for it breaking are kept to a minimum. What do you think?

This PR doesn't bump Python and conda for existing installs, so it shouldn't break anything. I tested this on my TLJH DO deployment and the Python and conda versions remained unchanged.

This PR just makes Python3.7 the default Python version used in TLJH (in the hub environment) and upgrades conda to a newer version, which also upgrades Python to 3.7 version in the user env (ref: #336).

Not sure if upgrading one at a time would be easier or less error prone (In the issue referenced in my initial comment there were people that tried to upgrade python separately and ended up with a broken env).

P.S> The tests were failing because I have forgotten to bump the conda version in tests also.

@yuvipanda
Copy link
Collaborator

Thanks for working on this, and for being so so patient, @GeorgianaElena!

There are two different Pythons:

  1. Python in the venv that runs the hub, running system python (currently 3.6)
  2. Default Python that comes with miniconda

I think we shouldn't touch the venv right now - there isn't any reason to, and we can bump it when we move from Ubuntu Bionic to the next LTS release when it comes out.

For the user environment, I think we should install newer miniconda (and with it, newer python), and not actually try to upgrade python of existing installs. This can break packages people have installed, esp with pip. If folks wanna upgrade their python, they can do so manually with a conda command I think.

So in summary, I think we should:

  1. Not update the venv python version
  2. Install newer miniconda with newer python only on newer installs
  3. Leave older miniconda installations alone, but provide documentation on how to upgrade python there

How does this sound?

@GeorgianaElena
Copy link
Member Author

Thanks @yuvipanda, @betatim for the review ❤️

I added new commits that:

  • leave the venv Python version as it is
  • check if miniconda 4.5.4 or 4.7.10 exist (and installs 4.7.10 if no miniconda)
  • add a new docs section that document how to manually upgrade Python for existing TLJH installations
    However, I'm not sure if the steps I provided can lead to trouble (I tried them and managed to upgrade) or if all of them are needed.

@yuvipanda
Copy link
Collaborator

@GeorgianaElena awesome! Minor suggestions, happy to merge after

@betatim
Copy link
Member

betatim commented Jan 31, 2020

Thanks for the work!

To clarify the "update one at a time" comment: I was thinking of two PRs, so that we can deal with build failures from changing things one at a time instead of having the combined build failures of two changes.

Co-Authored-By: Yuvi Panda <yuvipanda@gmail.com>
@yuvipanda yuvipanda merged commit af55717 into jupyterhub:master Jan 31, 2020
@yuvipanda
Copy link
Collaborator

Awesome! Thanks for your patience, @GeorgianaElena!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants