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

feat: upgrade docker image to py38 and add support for py39 #16889

Merged
merged 4 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/superset-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
if: steps.check.outcome == 'failure'
uses: actions/setup-python@v2
with:
python-version: "3.7"
python-version: "3.8"
- name: OS dependencies
if: steps.check.outcome == 'failure'
uses: ./.github/actions/cached-dependencies
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/superset-helm-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:

- uses: actions/setup-python@v2
with:
python-version: 3.7
python-version: 3.8

- name: Set up chart-testing
uses: ./.github/actions/chart-testing-action
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/superset-python-integrationtest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7]
python-version: [3.8]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down Expand Up @@ -77,7 +77,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7, 3.8]
python-version: [3.8, 3.9]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down Expand Up @@ -141,7 +141,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7]
python-version: [3.8]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/superset-python-misc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7]
python-version: [3.8]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v2
Expand Down Expand Up @@ -50,7 +50,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7]
python-version: [3.8]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v2
Expand All @@ -76,7 +76,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7]
python-version: [3.8]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/superset-python-unittest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7,3.8]
python-version: [3.8, 3.9]
env:
PYTHONPATH: ${{ github.workspace }}
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/superset-translations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: [3.7]
python-version: [3.8]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v2
Expand Down
6 changes: 3 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
######################################################################
# PY stage that simply does a pip install on our requirements
######################################################################
ARG PY_VER=3.7.9
ARG PY_VER=3.8.12
FROM python:${PY_VER} AS superset-py

RUN mkdir /app \
Expand Down Expand Up @@ -73,7 +73,7 @@ RUN cd /app/superset-frontend \
######################################################################
# Final lean image...
######################################################################
ARG PY_VER=3.7.9
ARG PY_VER=3.8.12
FROM python:${PY_VER} AS lean

ENV LANG=C.UTF-8 \
Expand All @@ -94,7 +94,7 @@ RUN mkdir -p ${PYTHONPATH} \
libpq-dev \
&& rm -rf /var/lib/apt/lists/*

COPY --from=superset-py /usr/local/lib/python3.7/site-packages/ /usr/local/lib/python3.7/site-packages/
COPY --from=superset-py /usr/local/lib/python3.8/site-packages/ /usr/local/lib/python3.8/site-packages/
# Copying site-packages doesn't move the CLIs, so let's copy them one by one
COPY --from=superset-py /usr/local/bin/gunicorn /usr/local/bin/celery /usr/local/bin/flask /usr/bin/
COPY --from=superset-node /app/superset/static/assets /app/superset/static/assets
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
# limitations under the License.
#

# Python version installed; we need 3.8 or 3.7
PYTHON=`command -v python3.8 || command -v python3.7`
# Python version installed; we need 3.7-3.9
PYTHON=`command -v python3.9 || command -v python3.8 || command -v python3.7`

.PHONY: install superset venv pre-commit

Expand Down Expand Up @@ -62,7 +62,7 @@ update-js:

venv:
# Create a virtual environment and activate it (recommended)
if ! [ -x "${PYTHON}" ]; then echo "You need Python 3.7 or 3.8 installed"; exit 1; fi
if ! [ -x "${PYTHON}" ]; then echo "You need Python 3.7, 3.8 or 3.9 installed"; exit 1; fi
test -d venv || ${PYTHON} -m venv venv # setup a python3 virtualenv
. venv/bin/activate

Expand Down
2 changes: 1 addition & 1 deletion RELEASING/Dockerfile.from_local_tarball
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
FROM python:3.7-buster
FROM python:3.8-buster

RUN useradd --user-group --create-home --no-log-init --shell /bin/bash superset

Expand Down
2 changes: 1 addition & 1 deletion RELEASING/Dockerfile.from_svn_tarball
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
FROM python:3.7-buster
FROM python:3.8-buster

RUN useradd --user-group --create-home --no-log-init --shell /bin/bash superset

Expand Down
2 changes: 1 addition & 1 deletion RELEASING/Dockerfile.make_docs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
FROM python:3.7-buster
FROM python:3.8-buster
ARG VERSION

RUN git clone --depth 1 --branch ${VERSION} https://github.com/apache/superset.git /superset
Expand Down
2 changes: 1 addition & 1 deletion RELEASING/Dockerfile.make_tarball
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
FROM python:3.7-buster
FROM python:3.8-buster

RUN apt-get update -y
RUN apt-get install -y jq
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/docs/installation/installing_scratch.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ We don't recommend using the system installed Python. Instead, first install the
brew install readline pkg-config libffi openssl mysql postgres
```

You should install a recent version of Python (Superset uses 3.7.9). We'd recommend using a Python version manager like [pyenv](https://github.com/pyenv/pyenv) (and also [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)).
You should install a recent version of Python (the official docker image uses 3.8.12). We'd recommend using a Python version manager like [pyenv](https://github.com/pyenv/pyenv) (and also [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv)).

Let's also make sure we have the latest version of `pip` and `setuptools`:

Expand Down
2 changes: 1 addition & 1 deletion requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ sqlalchemy==1.3.24
# flask-sqlalchemy
# marshmallow-sqlalchemy
# sqlalchemy-utils
sqlalchemy-utils==0.36.8
sqlalchemy-utils==0.37.8
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sqlalchemy-utils==0.36.8 has been yanked from PyPI, causing an ugly warning when installing deps. In addition, py39 was added to the officially supported versions right after 0.37.8 was cut (kvesteri/sqlalchemy-utils@b5b48b1), so I assume 0.37.8 to be fully supported by 3.9 in practice.

# via
# apache-superset
# flask-appbuilder
Expand Down
2 changes: 1 addition & 1 deletion requirements/development.in
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
-r base.in
flask-cors>=2.0.0
mysqlclient==1.4.2.post1
pillow>=7.0.0,<8.0.0
pillow>=8.3.1,<9
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pillow 7.x caused problems on Python 3.9

pydruid>=0.6.1,<0.7
pyhive[hive]>=0.6.1
psycopg2-binary==2.8.5
Expand Down
4 changes: 2 additions & 2 deletions requirements/development.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:e4f3ea65026a8aec3735d6d9977f89fef4a1a4f9
# SHA1:dbd3e93a11a36fc6b18d6194ac96ba29bd0ad2a8
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand Down Expand Up @@ -40,7 +40,7 @@ mysqlclient==1.4.2.post1
# via -r requirements/development.in
openpyxl==3.0.7
# via tabulator
pillow==7.2.0
pillow==8.3.1
# via -r requirements/development.in
progress==1.6
# via -r requirements/development.in
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def get_git_sha() -> str:
"simplejson>=3.15.0",
"slackclient==2.5.0", # PINNED! slack changes file upload api in the future versions
"sqlalchemy>=1.3.16, <1.4, !=1.3.21",
"sqlalchemy-utils>=0.36.6, <0.37",
"sqlalchemy-utils>=0.37.8, <0.38",
"sqlparse==0.3.0", # PINNED! see https://github.com/andialbrecht/sqlparse/issues/562
"tabulate==0.8.9",
"typing-extensions>=3.10, <4", # needed to support Literal (3.8) and TypeGuard (3.10)
Expand Down Expand Up @@ -169,5 +169,6 @@ def get_git_sha() -> str:
classifiers=[
"Programming Language :: Python :: 3.7",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Programming Language :: Python :: 3.7",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're still going to support 3.7 (we support last 3 python versions officially). Once 3.10 is out, we'll remove 3.7 support in a major version bump

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to deprecate 3.7 too, but I think we should give it some more time (it's still currently the recommended version) - let's leave it alive for a month or so so everyone can get upgraded to at least 3.8

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etr2460 @villebro do we—Apache Superset—have an official policy about supporting the last three Python versions? Just asking. If that's the case I think this is great as it means we're not unnecessarily burdened by older less feature rich versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@john-bodley @etr2460 I think we just lightly agreed on the last 3 versions on a community meeting. Making it an official policy is a good idea to let users know what to expect and plan ahead

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@john-bodley TBH until this PR we were only really supporting two 😄 But yeah, I think there was an unwritten agreement to try to support 3. I'm fine cutting it down to "at least 2" but trying to support 3 if possible.

"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
],
)
4 changes: 3 additions & 1 deletion superset/utils/memoized.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ def __get__(
if not self.is_method:
self.is_method = True
# Support instance methods.
return functools.partial(self.__call__, obj)
func = functools.partial(self.__call__, obj)
func.__func__ = self.func # type: ignore
return func
Comment on lines -67 to +69
Copy link
Member Author

@villebro villebro Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't quite able to figure out why this is necessary, but running doctests on memoized methods started failing on 3.9 due to the doctests failing to find the __func__ attribute on memoized methods. The only thing I found is this which seems like the same problem, but is said to affect Python 3.8, not 3.9: https://bugs.python.org/issue12790 Hacky workaround, but does the trick. I'll try to figure out how to solve this more elegantly before this gets merged

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird!



def memoized(
Expand Down