Redash: Migrate to Python 3

[arikfr]

As Python 2 reaches EOL, we're planning to have the next release of Redash to support Python 3.

Because Python 2 reaches EOL and the fact we don't have capacity to support both versions, once we switch to Python 3 we will support Python 3 only.

[arikfr]

Related: #2891.
Previous attempt: https://github.com/getredash/redash/tree/py3-tests.

[arikfr]

We will probably need to update some dependencies (either versions or replace) as outlined in #2891. But as we will be carefully testing everything, it's worth bumping the versions of other dependencies like moving to Flask 1.0+.

[NicolasLM]

Hi, here is the summary of my progress on this issue for the week:

• The most obvious syntax changes have been addressed throughout the code base, mostly using 2to3
• The Dockerfile has been updated to use Python 3.7
• The frontend build in CircleCI has been changed to also use Python 3
• A few dependencies have been upgraded because they were not compatible with Python 3. I tried to avoid upgrading unnecessary dependencies since I want to isolate the issues related to Python 3 from more generic backward incompatibilities that are often introduced in new versions of a package.
• I am working my way through the tests, fixing the broken ones. The vast majority of them already pass.

Currently the web app and the tasks work fine for me on Python 3. I have not encountered any error using the application locally, but I have only tested the Postgres and MySQL datasources. Feel free to give it a try if you want, the code is in the python-3 branch.

[arikfr]

Thank you for the detailed update, @NicolasLM. :+1:

[NicolasLM]

Only a single test remains broken: test_outdated_queries_works_scheduled_queries_tracker. After spending some time on it I feel that the migration to Python 3 uncovered a logic error in the test.
I am not sure which behavior is expected in the test as fixing the obvious comparison between TZ aware and naive datetime, makes the actual assert fail.

Besides that I also stopped using the CSV UnicodeWriter. I think that the clear distinction of str vs bytes in Python 3 makes this class redundant. If you could tell me why this class was needed in the first place I could verify that it is indeed not required anymore.

[arikfr]

Only a single test remains broken: test_outdated_queries_works_scheduled_queries_tracker. After spending some time on it I feel that the migration to Python 3 uncovered a logic error in the test.

I think you're right. I changed the test to use redash.utils.utcnow as the other tests in this group (#4230).

Besides that I also stopped using the CSV UnicodeWriter. I think that the clear distinction of str vs bytes in Python 3 makes this class redundant. If you could tell me why this class was needed in the first place I could verify that it is indeed not required anymore.

I believe that without it trying to write unicode into CSV failed. The implementation of UnicodeWriter was taken from csv docs for Python 2. I assume this is not needed anymore.

[NicolasLM]

Hi, regarding the port of the code to Python 3:

• All tests pass on Python 3
• The most common features of the application seem to work well.

I think that the code is ready for a review and thorough testing (specifically regarding data sources). Let me know if you want me to open a PR before the work on RQ is merged.

[arikfr]

This is exciting! :)

When I was running the tests locally I noticed that there were many warnings. Most seemed to be deprecation warnings. Some of them were due to issues in our code and some were due to older dependencies using some other dependencies in a deprecated way (mostly SQLA uses).

Considering we will put the system to a thorough testing due to the Python 3 migration, how about we upgrade our dependencies now that all tests are passing?

You can open the PR regardless, but we do need to decide whether we want to:

1. Merge Python 3 changes and then rebase the RQ branch.
2. Merge RQ changes and then rebase the Python 3 branch.

@rauchy wdyt?

[rauchy]

@arikfr I tend to lean towards #2, mostly because I don't think it's a complicated rebase, but also because I wouldn't want to postpone RQ from production much longer. I can take care of the rebase.

[arikfr]

Ok, let's aim to merge it soon then. If any problem arises when syncing the Python 3 brancn with these changes, will try to assist.

[NicolasLM]

I saw that the RQ branch was merged, so I rebased my work on master, fixed a few things and pushed a new branch python-3-rq. Everything seems to work well.

[rauchy]

@NicolasLM awesome, thank you!

[arikfr]

@NicolasLM super!

Any thoughts on the dependencies update? We can do it in two steps:

1. Merge Python 3 changes into master.
2. Update dependencies.

And after that put the whole thing into rigorous testing.

[NicolasLM]

Sounds like a good plan.

[arikfr]

Let's do it :-)

[jezdez]

OMG! I stepped away for a bit and now this! 🎉

[funkindy]

@NicolasLM i've just came across a little issue in python query runner. There are couple of py2 builtins (reduce, cmp, unicode) used here:
https://github.com/getredash/redash/blob/5cd6913e4085af500fe1da3425398335124c1514/redash/query_runner/python.py#L41
that breaks the process here:
https://github.com/getredash/redash/blob/5cd6913e4085af500fe1da3425398335124c1514/redash/query_runner/python.py#L239

would be great if you include this in v3 migration.

[arikfr]

@funkindy #4375.

[arikfr]

The Python 3 migration is over. We been running this in production for the past month and overall things look stable.