search

davidskalinder / mpeds-coder

MPEDS Annotation Interface
MIT License
0 stars 0 forks source link

Python 3 #116

Open davidskalinder opened 3 years ago

davidskalinder commented 3 years ago

Hahahahaha: https://www.python.org/doc/sunset-python-2/

So yeah turns out Python 2 hasn't been supported for a year. TBH we're not exactly depending on lots of new features, so it's not like staying on the unsupported version is holding us back, but running a web-facing application on a thing that won't be getting security updates is going to get dangerouser and dangerouser as new exploits are found.

So I guess in theory we should try to move the whole boat to v3. This guide, in addition to the official link above, seems like as good a starting point as any; and specifically we can check what'll need to be changed here.

With terrific luck, this could end up being only a medium-big job. (I can already think of a few places where the change to unicode will need attention; and I assume there's some division in there somewhere. So if it's only that stuff it'll a few days work at a minimum.) But of course it's easy to imagine this being a major undertaking. Interested parties should probably meet to discuss.

olderwoman commented 3 years ago

Yikes. Yeah, we should meet.

davidskalinder commented 3 years ago

Relevant to our interests: https://askubuntu.com/questions/1230615/ubuntu-20-04-still-supporting-python2

davidskalinder commented 3 years ago

Discussed this at today's team meeting. Takeaway is that for now we should spec it out to get an idea of how big a job it'll be.

davidskalinder commented 3 years ago

So @alexhanna confirms this is a low priority for her team unless a) it becomes necessary if/when she starts building pass 2 functionality into MAI this summer(ish), or b) something catches fire.

I'll start trying to identify what would need to be done for this.

davidskalinder commented 3 years ago

Okay I just ran 2to3 on our master branch and reviewed the diff. It looks like this all might not be too bad actually. I noted the following changes that will be better done manually:

After that, running 2to3 with the print fixer turned off should pretty nicely clean things up (or at least the things that 2to3 notices).

The other thing that is making me much more optimistic about this issue is that #65 makes it look like the number of Python dependencies is actually a pretty small list of well-maintained packages. I'm still a little nervous about whether there are some flask (bower?) components that might break with a new Python version... But the fact that #65 has now probably upgraded all the conda packages successfully gives me a lot of hope.

So, to do for this issue:

davidskalinder commented 3 years ago
  • Change the /etc/apache2/sites-enabled2 conf to add the new python path
    • This will make it safer to test on the backup VM, although maybe it's possible to do it on port 8080 of the main one? Or I could actually learn how apache works

Hmm: https://modwsgi.readthedocs.io/en/develop/user-guides/virtual-environments.html#virtual-environment-and-python-version

So unless there's a trick I haven't learned yet, it looks like we might have to reinstall mod_wsgi (somehow?) and upgrade all the sites at once?...

davidskalinder commented 3 years ago

reinstall mod_wsgi (somehow?) and upgrade all the sites at once?...

Looks like sudo apt-get install libapache2-mod-wsgi-py3 should do the trick?

davidskalinder commented 3 years ago

So unless there's a trick I haven't learned yet, it looks like we might have to reinstall mod_wsgi (somehow?) and upgrade all the sites at once?...

Hmm so the trick seems to be to install nginx and use it to like proxy two different apache sites and then route it through the Azores and translate the text into Vietnamese and back again and FSM knows what else. Sounds like it'll be a great idea to not do that.

Mind you, not doing that means that we need to use the backup VM to test the changes (and to figure out the new apache settings). And that means that #65 can't be fully tested while the backup VM is running Python 3 (unless @alexhanna's deployment on the backup VM is also running Python 3).

Having said that, it shouldn't be too difficult to switch the backup VM between v2 and v3 by simply running sudo apt-get install libapache2-mod-wsgi or sudo apt-get install libapache2-mod-wsgi-py3 (and probably switching the config files). So I think that's what I should do.

davidskalinder commented 3 years ago

Revised to-do list:

Lots of steps but they're awfully waterfall-y, so I think it's best to leave this all as a to-do list rather than split into issues (unless some of them start getting more complicated).

davidskalinder commented 3 years ago
  • Set up a new conda env with Python 3 and the requirements identified in #65

Hmph. conda create -n mai python flask pyyaml pandas lxml flask-login sqlalchemy MySQL-python uncomplainingly installs python 2.7, but conda create -n mai python=3 flask pyyaml pandas lxml flask-login sqlalchemy MySQL-python results in a bunch of dependency conflicts, I guess because some of these packages aren't up-to-date with the bleeding edge of everything in the conda repo(?)s.

Seems like there oughta be an option to bring everything as far up to date as possible, doesn't it? So I should try to find whether such a thing exists; if not I guess I'll just have to unpick them all one by one and figure out which versions to specify for everything...

davidskalinder commented 3 years ago

Hmph. conda create -n mai python flask pyyaml pandas lxml flask-login sqlalchemy MySQL-python uncomplainingly installs python 2.7, but conda create -n mai python=3 flask pyyaml pandas lxml flask-login sqlalchemy MySQL-python results in a bunch of dependency conflicts, I guess because some of these packages aren't up-to-date with the bleeding edge of everything in the conda repo(?)s.

Hmm, but conda create -n mai python=3 flask pyyaml pandas lxml flask-login sqlalchemy worked just dandy. I guess MySQL-python was a python 2 thing that held back the whole show.

So now I expect that either some other package now handles what MySQL-python used to or that MAI will choke when run in the new env because it needs something to handle MySQL. If it's the second one then there's probably some new package that SQLAlchemy(?) uses to do this that will need to be installed?

This also makes me wonder if we can get away without flask-login. I don't think we import it explicitly, and it looks an awful lot like the kind of thing that would get phased out in a new version. If I can get MAI working with py3 at all then maybe it'll be worthwhile to try running it in a clean env with nothing but python 3 installed and then install only the dependencies that get reported missing in the error logs...

Anyway, this bullet is provisionally done I think, though of course I may need to come back to it if it breaks subsequent steps...

davidskalinder commented 3 years ago
  • Set up a new conda env with Python 3 and the requirements identified in #65

Okay so this did indeed break MySQL, so it needs a new connector; apparently the one to use these days is mysqlclient.

There also seems to be trouble when the version of python installed in the conda env doesn't match the version that mod_wsgi provides (since what apache actually runs is a python binary bundled into mod_wsgi, and then we inject the conda env's modules into the path to fool apache into using those instead of whatever it would normally). So MAI was choking when the conda env had python 3.9 and mod_wsgi had 3.8 (the mod_wsgi version is visible by restarting apache and checking the error log.) This means that going forward (forever?) we'll need to make sure that the conda python version matches (or probably just doesn't exceed) mod_wsgi's version.

So this works to resolve those problems:

conda create -n mai python=3.8 flask pyyaml pandas lxml flask-login sqlalchemy mysqlclient

After this, the site works, but adding an event block fails. Time for some more debugging.

davidskalinder commented 3 years ago

adding an event block fails

Because there's python 2 code in the jinja templates that needs changing.

Meanwhile, I'm being sidetracked by some strange git issues that I'll need to run down before getting back to the python debugging...

davidskalinder commented 3 years ago

Back on track:

Because there's python 2 code in the jinja templates that needs changing.

Fixed in 29b11dcce4e6.

Now everything in the coder UI seems to work except for the test captures, ominously...

davidskalinder commented 3 years ago

Now everything in the coder UI seems to work except for the test captures, ominously...

This has gotta be more of the python calls in the template (rather than any JS issues, since JS doesn't care about python versions). There's a {% for k,v in vars %} that I don't like the look of, but it turns out that vars in this context is a list of tuples and not a dict, so I'm not sure why that's failing. Maybe k,v is now required to be (k, v)? Though I doubt it since I think a similar call in the yes-no section also has a naked tuple like that...

Anyway out of time for this for now, will have to come back later...

davidskalinder commented 3 years ago

Just realized that my last several commits for this have been in @alexhanna's name due to the issue with git users following the Ubuntu upgrade.

So I need to get that sorted out before this can progress (and I think it's almost done). Once it is, I think the best thing will be to simply build a new branch, copy the changes, and recommit them, so that we don't get the users all mixed up in case we need to review the changes later...

davidskalinder commented 3 years ago

Just realized that my last several commits for this have been in @alexhanna's name due to the issue with git users following the Ubuntu upgrade.

So I need to get that sorted out before this can progress (and I think it's almost done). Once it is, I think the best thing will be to simply build a new branch, copy the changes, and recommit them, so that we don't get the users all mixed up in case we need to review the changes later...

All right I have now hand-repainted these commits, pushed, and pulled to the other server. So it looks like this can get back on track now.

davidskalinder commented 3 years ago

Now everything in the coder UI seems to work except for the test captures, ominously...

This has gotta be more of the python calls in the template (rather than any JS issues, since JS doesn't care about python versions). There's a {% for k,v in vars %} that I don't like the look of, but it turns out that vars in this context is a list of tuples and not a dict, so I'm not sure why that's failing. Maybe k,v is now required to be (k, v)? Though I doubt it since I think a similar call in the yes-no section also has a naked tuple like that...

Anyway out of time for this for now, will have to come back later...

$%"£%(%*)! I was using Chrome instead of Firefox (since I'm testing on the backup VM that's only available on the SSCC network and Winstat only has Chrome). So I'm guessing that this has been working just fine all along, ackshully.

Still though, it'd be nice to test this properly rather than having to wait until switching the production server to Python 3 and holding our breaths. I'll see if I can figure out a way to test on the backup VM with FF...

davidskalinder commented 3 years ago

I'll see if I can figure out a way to test on the backup VM with FF...

All right, daisy-chaining a few SSCC resources together did the trick! And it all looks good for text capture with FF.

So I think this (at 5191a290777a541b) might be all that's needed for getting upstream to v3? So the remaining tasks are:

I don't think there's much appetite for user testing for our project since MAI is dormant right now for us; so that means that after this I think all that's left to do is to PR the changes, deploy (without restarting apache), and then install the new WSGI package (which will restart apache and launch the py3 version)? Though I should doublecheck the steps above before we get to that point.

davidskalinder commented 3 years ago
  • Deploy the changes to @alexhanna's deployment on the backup VM so she can test them

Okay I'm now realizing this probably isn't the best thing to do since I don't fully understand what's going on in that deployment (on either the main or backup VM).

I think instead the best way for me to proceed will be for me to PR the thing and let @alexhanna deploy it to the backup VM (whose mod_wsgi is currently running Python 3) to test it as she likes before deploying to the production server.

Meanwhile, I also need to set up a Python 3 conda env on the production server...

davidskalinder commented 3 years ago

after this I think all that's left to do is to PR the changes, deploy (without restarting apache), and then install the new WSGI package (which will restart apache and launch the py3 version)?

Also, need a new sites-enabled file to point the new conda env. Best way to do this is to keep both the old one and the new one and swap to the new one before installing wsgi3.

davidskalinder commented 3 years ago
  • Make a py3 branch for article-level coding, merge in the py3-upstream branch, run 2to3 again, and test again (possibly changing some more templates)

Uh mkay and now I'm realizing that this isn't a good idea either, because in theory all the branches that aren't upstream yet might need changes. So the proper way to do this will be to wait until @alexhanna accepts all the outstanding PRs, pull the new upstream, merge it into the py3_upstream branch, and then PR those changes. And even then there might be stuff that's not upstream yet which would require further changes...

So maybe the answer is that this issue depends on #119, since that will ensure that there will be only one branch that needs updating...

alexhanna commented 3 years ago

Revisiting this thread. I think this is ready to go if you want to PR the Python 3 changes.

davidskalinder commented 3 years ago

Revisiting this thread. I think this is ready to go if you want to PR the Python 3 changes.

Rad, thanks. I might not get to this today but hopefully v soon.