Skip to content

Commit

Permalink
Add CircleCI jobs to run the oldest and newest Python versions we sup…
Browse files Browse the repository at this point in the history
…port. (#1053)

As part of this, we've removed the per-Python-version lib/Pipfile.locks/
files in favor of running without pipenv lock files, and having CircleCI
cache our Python environment using a combination of the Python binary,
lib/Pipfile, and the current data.

This implies that once per day we'll potentially upgrade our Python
packages, consistent with any requirements in lib/Pipfile. We will
always regenerate our Python environment if lib/Pipfile changes,
potentially upgrading any other packages at that time as well. This was
deemed acceptable in a conversation with @tvst.

This also introduces tests.testutils.requires_tensorflow(), a decorator
for tests that require Tensorflow, so that we can skip these under
Python 3.8, which Tensorflow does not yet support.

In doing this work, it was discovered that we had broken our support for
Python 3.5. This required two fixes:

o Invocations of "black" were modified to tell it that it needs
  to retain support for Python 3.5 syntax, which does not permit a
  trailing comma after "**kwargs" in a function definition. Fixed this in
  two places in DeltaGenerator.py, restoring Python 3.5 support.

o In e2e/scripts/st_magic.py, tests of async generators were made
  conditional on Python 3.6 or greater.

Following this change I expect one follow-up commit, and I made a few
voluntary style changes:

o Once this makes it to master, I expect to change the
  "fully-specified" job from python-3.7.4 to python-3.8, leaving only
  one "inheriting" job at 3.5. Not doing this yet so we can get the new
  name passing before we change out GitHub status checks to reference it.

o The new job names reference the Python major and minor versions they
  belong to, but *not* the micro version (e.g., 4 in 3.7.4). I think we
  should be able to advance along these micro/maintenance releases
  without switching job names.

o In the new 3.5 and 3.8 jobs, I removed explicit selection of the
  Debian version underlying the circleci Debian image we run the tests
  under. I could just have easily switched these to "buster", the new
  latest Debian, but I think it's probably better to just go with
  CircleCI's defaults rather than having to remember to check and update
  this every year or so.

o Moved jstests to run against 3.8, since I'm planning to retire testing
  against 3.7 soon per the above. I tried doing this with Cypress as
  well, but ran against diffs due to font differences, so I fell back.
  • Loading branch information
Nate White committed Feb 10, 2020
1 parent 1c127ec commit 1b92148
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 3,718 deletions.
45 changes: 30 additions & 15 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ workflows:
circleci:
jobs:
- python-3.7.4
- python-3.5 # Oldest supported Python minor version.
- python-3.8 # Latest supported Python minor version.
- cypress-approval:
type: approval
requires:
Expand Down Expand Up @@ -49,14 +51,14 @@ jobs:
rm -f dot.bin
fi
- run:
# We use the python Pipfile.lock as the cache key and also
# the hash of the current python.
# If dependencies haven't changed then use the cache.
name: Create Python cache key
- run: &create_python_cache_key
# Combine hashes of the Python interpreter and Pipfile plus today's
# date into a file whose checksum will key the Python environ. cache.
name: Create Python environment cache key
command: |
md5sum lib/Pipfile.locks/${CIRCLE_JOB} > ~/python_version.md5
md5sum $(which python) >> ~/python_version.md5
md5sum $(which python) > ~/python_version.md5
md5sum lib/Pipfile >> ~/python_version.md5
date +%F >> ~/python_version.md5
- run: &create_yarn_cache_key
name: Create Yarn cache key
Expand Down Expand Up @@ -169,7 +171,8 @@ jobs:
python -m venv venv
source venv/bin/activate
make setup
make pipenv-lock
pip install --upgrade pip
make pipenv-install
deactivate
else
echo 'Virtualenv already exists, not creating'
Expand Down Expand Up @@ -233,17 +236,20 @@ jobs:
make integration-tests
#################################################################
# Run frontend tests. (Only executed in the Python 3 job.)
# Run frontend tests. (Only executed in one job.)
#################################################################
- run:
name: Run frontend tests
command: |
make jstest
if [ "${CIRCLE_JOB}" != "python-3.8" ] ; then
echo "Not running frontend tests because our job is ${CIRCLE_JOB}"
else
make jstest
fi
#################################################################
# Save cache for python virtualenv, node_modules, protobuf
#################################################################

- save_cache:
name: Save virtualenv to cache
key: v12-python-venv-{{ checksum "~/python_version.md5" }}
Expand All @@ -264,6 +270,18 @@ jobs:
- frontend/src/autogen/proto.js
- lib/streamlit/proto

# The following jobs inherit from python-3.7.4. In a few cases, there are
# steps that are skipped based on the name of the current job (see, e.g.,
# "Run frontend tests").
python-3.8:
<<: *job-template
docker:
- image: circleci/python:3.8.1
python-3.5:
<<: *job-template
docker:
- image: circleci/python:3.5.9

cypress:
docker:
- image: circleci/python:3.7.4-stretch
Expand All @@ -284,10 +302,7 @@ jobs:
<<: *get_dot_checksum

- run:
name: Create python cache key.
command: |
md5sum lib/Pipfile.locks/python-3.7.4 > ~/python_version.md5
md5sum $(which python) >> ~/python_version.md5
<<: *create_python_cache_key

- run:
<<: *create_yarn_cache_key
Expand Down
42 changes: 14 additions & 28 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ SHELL=/bin/bash
PYTHON_MODULES := $(foreach initpy, $(foreach dir, $(wildcard lib/*), $(wildcard $(dir)/__init__.py)), $(realpath $(dir $(initpy))))
PY_VERSION := $(shell python -c 'import platform; print(platform.python_version())')

# Configure Black to not depend on syntax only supported by Python >= 3.6.
BLACK=black --target-version=py35


.PHONY: help
help:
@# Magic line used to create self-documenting makefiles.
Expand Down Expand Up @@ -46,24 +50,6 @@ pipenv-install: lib/Pipfile
@# Runs pipenv install; doesn't update the Pipfile.lock.
cd lib; pipenv install --dev

.PHONY: pipenv-lock
# Re-generate Pipfile.lock. This should be run when you update the Pipfile.
pipenv-lock: lib/Pipfile
@# Regenerates Pipfile.lock and rebuilds the virtualenv. This is rather slow.
# In CircleCI, dont generate Pipfile.lock This is only used for development.
ifndef CIRCLECI
cd lib; rm -f Pipfile.lock; pipenv lock --dev && mv Pipfile.lock Pipfile.locks/python-$(PY_VERSION)
else
echo "Running in CircleCI, not generating requirements."
endif
cd lib; rm -f Pipfile.lock; cp -f Pipfile.locks/python-$(PY_VERSION) Pipfile.lock
ifndef CIRCLECI
# Dont update lockfile and install whatever is in lock.
cd lib; pipenv install --ignore-pipfile --dev
else
cd lib; pipenv install --ignore-pipfile --dev --system
endif

.PHONY: pylint
# Run "black", our Python formatter, to verify that our source files
# are properly formatted. Does not modify any files. Returns with a non-zero
Expand All @@ -73,11 +59,11 @@ pylint:
@# Black requires Python 3.6+ to run (but you can reformat
@# Python 2 code with it, too).
if command -v "black" > /dev/null; then \
black --check docs/ ; \
black --check examples/ ; \
black --check lib/streamlit/ --exclude=/*_pb2.py$/ ; \
black --check lib/tests/ --exclude=compile_error.py ; \
black --check e2e/scripts/ ; \
$(BLACK) --check docs/ ; \
$(BLACK) --check examples/ ; \
$(BLACK) --check lib/streamlit/ --exclude=/*_pb2.py$/ ; \
$(BLACK) --check lib/tests/ --exclude=compile_error.py ; \
$(BLACK) --check e2e/scripts/ ; \
fi

.PHONY: pyformat
Expand All @@ -87,11 +73,11 @@ pyformat:
@# Black requires Python 3.6+ to run (but you can reformat
@# Python 2 code with it, too).
if command -v "black" > /dev/null; then \
black docs/ ; \
black examples/ ; \
black lib/streamlit/ --exclude=/*_pb2.py$/ ; \
black lib/tests/ --exclude=compile_error.py ; \
black e2e/scripts/ ; \
$(BLACK) docs/ ; \
$(BLACK) examples/ ; \
$(BLACK) lib/streamlit/ --exclude=/*_pb2.py$/ ; \
$(BLACK) lib/tests/ --exclude=compile_error.py ; \
$(BLACK) e2e/scripts/ ; \
fi

.PHONY: pytest
Expand Down
3 changes: 1 addition & 2 deletions lib/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ url = "https://pypi.org/simple"
verify_ssl = true

[dev-packages]
funcsigs = {version = "*",markers = "python_version < '3.0'"}
mock = "*"
pipenv = "*"
pytest = "*"
Expand All @@ -24,7 +23,7 @@ bokeh = "*"
graphviz = "*"
parameterized = "*"
pydot = "*"
tensorflow = ">=2.0.0"
tensorflow = {version = ">=2.0.0", markers = "python_version < '3.8'"}
seaborn = "*"
prometheus-client = "*"
opencv-python = "*"
Expand Down
5 changes: 0 additions & 5 deletions lib/Pipfile.locks/README.md

This file was deleted.

0 comments on commit 1b92148

Please sign in to comment.