Upgrade to Python 3.11 #70
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wildest part of this was realizing that 2016 was almost 10 years ago
In this commit - brought some more packages up to date due to dependency issues - commented out pub sub stuff on gcp since don't have credentials (could test w my own, but not high priority) - fixed more issues resulting from upgrading to django 5 - fixed scraper to work with new query param schema and additional column (for lang requirements) - some other python 2 -> 3 changes/fixes Still to do: - analytics panel - fix password resets / add it back - test/make sure emails work (v important)
Replace u.is_admin w/ u.is_superuser
some of the styling could be better (uses buttons instead of links now), but not high priority
- fix instance of deprecated User.objects.make_random_password - update python version in runtime.txt used by heroku at buildtime
… has to do with another package b/c tried every single variation of settings outline in their docs and still nothing
(use version that replaces universal reads ('rU') with standard reads ('r') since 'rU' was deprecated in Python 3)
and remove yuglify binary locations
if it works, great. if not, i'll just remove the sourcemappingURL from the previous version's (1.11.3) minified file since it's just static content anyway. - also update filename of jquery-highlight
This reverts commit d143d80.
This reverts commit 3161008.
after this i should try to fix root cause
should work, but doesn't fix underlying issue: - when an update is made to requirements.txt, collectstatic using pipeline will not work until a change is deployed without using pipeline and the post_compile script installs nodejs and yuglify (since the install scripts don't seem to work before — as I'm typing this out I realized why and how to fix)
… it doesn't exist, so symlink doesn't work, which is what was breaking yuglify install step
also potentially fix the issue that made us add the jank path to our yuglify_binary in pipeline settings
This reverts commit 78b69ec.
This reverts commit 4629591.
This reverts commit 37681db.
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upgrade to Python 3.11
...and Node 18, Django 5.1, Bootstrap 5.3, jQuery 3.7, Celery 5.4, Gunicorn 23, etc.
Motivation
There were a couple of new(er) classes I wanted to take next term, but couldn't find any reviews about them or their professors. Since Layup List stopped working for new emails and terms, the rate of reviews uploaded to it and similar sites has massively slowed. Asked a couple of friends why they thought it was happening, and turns out most were still using the original Layup List (often just with other people's emails).
After that, I saw the code was open-source, stumbled across #69, and thought why not try.
Implementation
Commit 1e0d83b contains the bulk of the Python updates. Most of the dependencies are brought up to their most recent stable releases. I started by just trying to build the app using a Python 3.11 environment, and just knocked out each error I was getting one-by-one. As for the syntax changes, 3.11 is pretty well-documented online, so GPT helped a fair bit with telling me what I might want to change. This was actually the most straightforward part of the update.
Also had to make some changes to the scraper since part of the page and parameter structure has been changed (pull #66 tried to do the same thing). This was important since I wanted to make sure LL was able to properly scrape new classes (see motivations). See commit 4319a12 for more specificity.
The above (through commit 439678b) got the site working locally (with hijacking), but @zack-overflow recommended I try building it on Heroku (where the app is hosted) before making a PR. As a result, most of the following changes deal with updates to static files, build scripts, Django Pipeline, and Node package versioning issues.
Fixing Django Pipeline was one of the more tedious tasks, since the site would build (and run decently well!) without compressing all stylesheets and JS into two individual files (
app.jsandapp.css). Most of the commits should probably be squashed since it was just me testing different variations of the instructions outlined in Pipeline's documentation, which are out of date following the release of Django 5.1. The expectedsettings.pystructure changed in v5.1, meaning we're now setting the staticfiles storage inSTORAGESrather thanSTATICFILES_STORAGE(see commit 256376b).Once Pipeline was working, I could properly debug the Heroku build scripts located in
bin/(i.e., the JavaScript runtime, Yuglify, etc.). It looks like the way heroku-buildpack-python compiles apps changed over the past ten years (surprise), so this PR moves Node and Yuglify installations to thepre_compilescript, which runs beforepython manage.py collectstatic(see commit 6044d16). Then, I just updated some of the JavaScript dependencies (e.g., jquery-highlight, jquery).Since we updated jQuery, Bootstrap v3 was no longer being supported causing some (somewhat minor bugs). As a result, this PR also upgrades to Bootstrap v5.3 and handles virtually all of the restyling that v5 required (see 14e9d8f). The version specified by 65bd848 is the final working version of LL using Bootstrap v3 if that's preferred (see Heroku build log). Even though Bootstrap 5 made a lot of visual changes to some components, I tried to stay consistent with the general style of the site (even if it meant straying from a few button defaults).
PyReact (deprecated) also needed some updating since it was written in Python 2 and used universal reads (
rU), so I updatedrequirements.txtto include a fork that fixes all of the Python 2 and transpiling issue.Along the way, there were a few issues I ran into simply because the contributing guidelines were out of date, so I revised some of the installation instructions in
CONTRIBUTING.mdto be clearer. See commits 94542f1 and 4e45924.Usage
These changes are live, deployed on Heroku at https://layup-list-a10b5d7eb6ed.herokuapp.com/. Also see the final build log.
Since account creation doesn't work on prod (see below, no SMTP credentials), log in with these credentials to see auth-walled pages:
[email protected]R3v1v3 Layup Li5t!Considerations
UserManagerrow in my database. If I commented out queries with that user, it works as expected (with hijacking). Assuming the production database has this row, so left it in.Demo (screenshots, animations)
Screenshots are nice, but a live site where you can see the changes yourself is better. Use the account mentioned above to see and post reviews.
Referenced Issue