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

Migrate to Python 3 #39

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Migrate to Python 3 #39

wants to merge 8 commits into from

Conversation

psvenk
Copy link
Member

@psvenk psvenk commented Nov 27, 2022

Closes #38. This is very much not yet ready to merge. This seems to be working from some preliminary testing, but more testing is needed.

Some changes correspond to changes between Python 2 and Python 3 (e.g., import semantics, stdlib package layout, indentation enforcement); others correspond to changes in Django. I also moved the requirements to a requirements.txt file and removed the old version numbers (the listed Django version is incompatible with modern versions of Python anyway).

@psvenk psvenk marked this pull request as ready for review November 29, 2022 16:59
@cjquines
Copy link
Member

cjquines commented Dec 1, 2022

is fireroad-server run on its own server, or somewhere else? athena and scripts, for example, have a quite outdated version of python3

So far, enough things have been changed so that the setup.sh script
successfully migrates the database on Python 3.10.
All strings are Unicode by default in Python 3
is_authenticated is no longer a function
This should complete the migration to Python 3.
For some reason, 2to3 created a regression here.
@psvenk
Copy link
Member Author

psvenk commented Dec 3, 2022

This should be pretty close to ready to merge now. I tested with CourseRoad, Hydrant, and catalog updates, and the Python 3 version exhibits identical behavior to the Python 2 version (on master) in all cases. The test suite still fails on five cases, but this is not a regression.

@venkatesh-sivaraman
Copy link
Collaborator

Awesome work @psvenk!

The server runs on an IST managed server which should have a Python 3 installation (IIRC there's a virtualenv defined for FireRoad). Have you tested this on the dev server yet? If not I'd definitely recommend doing that before merging, since I suspect database migrations with Django will be a headache. You might want to even make notes of the steps you take to set it up on dev, to make the process smoother on prod. (I believe I tried to do a Python 3 conversion a while back, and ran into too many roadblocks for it to be worth continuing, which is why I mention this!)

Also, if all of that goes well, you might consider reaching out to the IST team to let them know you're making this change and to make sure there is an available backup of the prod database :)

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.

Python 3 support
3 participants