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

WIP: Preliminary investigation: updating to python 3 #1407

Closed

Conversation

drewsilcock
Copy link
Contributor

So far:

  • Moved dependencies over to Poetry (just made the process much easier with proper dependency resolution, doesn't need to be permanent change).
  • Run 2to3 and hand-pick output to check for false positives. (Haven't run through every line of code to check for false negatives).
  • Initial changes to fix immediate issues (blocked by not being able to import reporters_db currently, there's bound to be more issues after that though).
  • Upgrade dependencies as required to get them installed on py3.8.

When porting code I tend to take this approach:

  1. Upgrade dependencies until they install
  2. Try to run code
  3. It breaks
  4. Fix where it breaks
  5. Try to run code again
  6. It breaks in a different place
  7. Fix where it breaks again
  8. Run it again: it breaks => goto 6. else continue
  9. It works
  10. Celebrate
  11. Ah but this bit doesn't work... => goto 6
  12. You get the idea

Next steps:

  • Use mypy for an additional level of checking to find more issues without needing to run every line of code.
  • Getting reporters_db installed and working on py3 so that we can continue with the porting cycle.

If you want to discuss any of these changes and potential problems, here's a good place to do it! 🙂

Add new docker image to CL
Add simple test to begin replacing weird binaries
Use our new docker container to generate the thumbnail
and return the contents
Refactor subprocess into our new BTE container
Not sure its necessary or used in any way
The method only extracts page count for PDFs
prior to this refactor.
Add volumes to new containers to task server
Remove unneeded installs to task server
Currently tests.yml is not running
this is an attempt to get it working in
a new syntax
Built and pushed beta containers for Django
and the celery task-server.

Task server is imported from django image and
neither has the 'weird binaries'.

The new images are 36% of the old ones
Add JUDGE_PICS_PATH
add SEAL_ROOKERY_PATH
Add BTE URL to settings and point to it in functions
flooie and others added 23 commits October 5, 2020 17:10
Include changes to migration files help text encoding
use // instead of / for integer division
In cleanup main query and get related clusters with cache
replaced sunburnt with scorched
Upgrade scorched, ipython, cryptography and comment out psycopg2
Remove old magic code with python magic, add to req.
Where pip needs `==a.b.*,>=a.b.c`, poetry can just do `^a.b.c`. Strictly
speaking this isn't exactly the same as `^a.b.c` will satisfy any
`a.d.e` where d > b and e > c. This makes the versioning more reliable,
because the pip version could install major package updates without you
realising.
@drewsilcock
Copy link
Contributor Author

I've pushed an update to replace mock with unittest.mock and remove the cl/cleanup directory, although it might make more sense to put those up as a PR against the python38-merger branch?

@drewsilcock
Copy link
Contributor Author

p.s. Python 3.9 is now the latest stable version of Python, as of Monday. (Not that it's particularly important, just thought I'd mention.)

@mlissner
Copy link
Member

We're incorporating these changes into our weirdly-named 1407-py3-unmerge branch and it has a bunch more changes, so I'm closing this one. Thanks for kicking this off, Drew. Huge deal.

@mlissner mlissner closed this Oct 13, 2020
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.

3 participants