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

Minimum version dependency testing #6802

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
8ee49d7
Update dependency version constraints
AnOctopus Jun 5, 2023
528c688
Add generated min constraints and generation script
AnOctopus Jun 5, 2023
b2a3b3b
Merge branch 'develop' into feature/minimum-dependency-version-testing
AnOctopus Jun 6, 2023
a111d88
Merge branch 'develop' into feature/minimum-dependency-version-testing
AnOctopus Jun 8, 2023
7ab31d4
Ensure consistent order of minimum version constraints
AnOctopus Jun 8, 2023
37b9b5f
Save reordered constraints file
AnOctopus Jun 8, 2023
1d4beee
Copied workflow, using generated min constraints
AnOctopus Jun 12, 2023
d2f26c2
Pull in mypy-protobuf for generating in min tests
AnOctopus Jun 12, 2023
d701597
Ignore potential type error in script
AnOctopus Jun 12, 2023
400ea90
Merge branch 'develop' into feature/minimum-dependency-version-testing
AnOctopus Jun 12, 2023
90d223e
Merge branch 'develop' into feature/minimum-dependency-version-testing
AnOctopus Jun 13, 2023
8d5211e
Remove duplicate "install" in make command
AnOctopus Jun 15, 2023
7484e7e
Fix integration test failures
AnOctopus Jun 19, 2023
c335cd8
CI failure debugging
AnOctopus Jun 19, 2023
ab013bb
More debugging
AnOctopus Jun 19, 2023
4ba92fb
debug
AnOctopus Jun 19, 2023
0bad70a
print tests module location
AnOctopus Jun 19, 2023
0c49d61
debug
AnOctopus Jun 19, 2023
8b5af0c
pip list
AnOctopus Jun 20, 2023
af96928
Print sys.path
AnOctopus Jun 20, 2023
bc63854
Debug path in script
AnOctopus Jun 20, 2023
62a6946
more path debugging
AnOctopus Jun 20, 2023
3852266
Merge branch 'develop' into feature/minimum-dependency-version-testing
AnOctopus Jun 20, 2023
c808c48
Update typing-extensions for compatibility with snowflake-connector-p…
AnOctopus Jun 20, 2023
f8f5d2d
Does the problem happen if we install everything?
AnOctopus Jun 21, 2023
662158c
Closer to develop
AnOctopus Jun 21, 2023
d574e56
Maybe it is the followup make develop that lets it work
AnOctopus Jun 21, 2023
de17c61
Use real name
AnOctopus Jun 21, 2023
30ffb8a
Remove debug stuff
AnOctopus Jun 22, 2023
c1eef1e
Inline the make_init action
AnOctopus Jun 22, 2023
d68275c
Revert action for main python test jobs
AnOctopus Jun 22, 2023
990d138
Add documentation about the inlined make_init section
AnOctopus Jun 22, 2023
6bfb8f0
Add make command for generating min constraints
AnOctopus Jun 22, 2023
c56b9ec
See if this fixes the conda build
AnOctopus Jun 22, 2023
3a411f7
Allow altair test to pass with <4.2
AnOctopus Jun 23, 2023
e7045fd
Dead code elimination, mark end of inlined section
AnOctopus Jun 26, 2023
3482e02
Document related inlined code
AnOctopus Jun 26, 2023
42819e6
Add comment about invoking the min versions script
AnOctopus Jun 26, 2023
0fddccd
Merge branch 'develop' into feature/minimum-dependency-version-testing
AnOctopus Jun 26, 2023
7bb3f58
Formatting
AnOctopus Jun 26, 2023
b8156ce
Use different concurrency group for new workflow
AnOctopus Jun 26, 2023
dece0f4
Fix altair version check
AnOctopus Jun 26, 2023
afd05e2
Try doubling up?
AnOctopus Jun 26, 2023
61bb0fb
Formatting
AnOctopus Jun 26, 2023
4068dcd
Try installing deps just once
AnOctopus Jun 26, 2023
a0e9582
Debugging cleanup
AnOctopus Jun 26, 2023
33a7e54
f-strings and default case handling in script
AnOctopus Jun 26, 2023
5347517
Add min version constraints validation step to workflow
AnOctopus Jun 26, 2023
35d86ef
Remove invalid else branch, add comment
AnOctopus Jun 26, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/actions/make_init/action.yml
Expand Up @@ -6,6 +6,8 @@ inputs:
description: "Use Cached Virtual Env"
default: "true"

# There is an altered copy of these steps inlined in the python_min_deps
# workflow. If you make changes to this action, apply them there too if appropriate.
runs:
using: composite
steps:
Expand Down
178 changes: 178 additions & 0 deletions .github/workflows/python-min-deps.yml
@@ -0,0 +1,178 @@
name: Python Dependency Minimum Versions

on:
push:
branches:
- "develop"
- "feature/**"
pull_request:
types: [opened, synchronize, reopened]
# Allows workflow to be called from other workflows
workflow_call:
inputs:
ref:
required: true
type: string
secrets:
PARAMETER_PASSWORD:
description: "Token passed from caller workflows for snowflake integration tests"
required: true

# Avoid duplicate workflows on same branch
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-python-min
cancel-in-progress: true

defaults:
run:
shell: bash --login -eo pipefail {0}

env:
FORCE_COLOR: "1"

jobs:
build_info:
runs-on: ubuntu-latest

name: "Build info"

steps:
- name: Checkout Streamlit code
uses: actions/checkout@v3
with:
ref: ${{ inputs.ref }}
persist-credentials: false
submodules: "recursive"
fetch-depth: 2
- name: Set Python version vars
id: build_info
uses: ./.github/actions/build_info
with:
force-canary: ${{ false }}

outputs:
PYTHON_MIN_VERSION: ${{ steps.build_info.outputs.PYTHON_MIN_VERSION }}

py_version:
needs:
- build_info

runs-on: ubuntu-latest

strategy:
fail-fast: false

# TODO: Should we add developer-friendly name for this job also?
# This will unfortunately require a branch protection update.

env:
PYTHON_VERSION: "${{needs.build_info.outputs.PYTHON_MIN_VERSION}}"

steps:
- name: Checkout Streamlit code
uses: actions/checkout@v3
with:
ref: ${{ inputs.ref }}
persist-credentials: false
submodules: "recursive"

- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@v4
with:
python-version: ${{ env.PYTHON_VERSION }}

# This section is an inlined copy of the make_init action
# Both because we need to do some parts of it differently
# and because it running in a separate action causes the python path
# setup to work weirdly, so some tests fail unless you run an install
# command in the workflow directly.
Comment on lines +84 to +88
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may have already experimented with this, but just to confirm - adding a new input flag to the make_init action to handle the differences below doesn't work?

List of those I could identify:

  • creating the Python environment cache key (lib/min-constraints-gen.txt vs. lib/dev-requirements.txt)
  • restore virtualenv from cache (changed key name)
  • Create Virtual Env (make python-init-test-min-deps vs. make python-init)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't try doing it as a new input flag, because it felt like it would result in a bad abstraction. While prototyping this, I made changes to make_init directly, and then when I discovered that the path only gets set up correctly if you do an additional install command in the workflow (the existing test workflows do make develop in a step after the make_init action), I decided to inline the action's steps.

- name: Restore pre-commit cache
id: cache-pre-commit
uses: actions/cache@v3
with:
path: ~/.cache/pre-commit
key: v1-pre-commit-${{ env.pythonLocation }}-${{ hashFiles('**/.pre-commit-config.yaml') }}
- name: Install pre-commit
run: |
pip install pre-commit
pre-commit install-hooks
- name: Setup Node
uses: actions/setup-node@v3
with:
node-version-file: ".nvmrc"
cache: "yarn"
cache-dependency-path: "**/yarn.lock"
- name: Initialize React
run: |
# Create the cache directory if it does not exist.
mkdir -p $(yarn cache dir)
make react-init
- name: Install system dependencies
run: |
echo "deb http://ppa.launchpad.net/maarten-fonville/protobuf/ubuntu trusty main" | sudo tee /etc/apt/sources.list.d/protobuf.list
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 4DEA8909DC6A13A3
sudo apt update
# protobuf, pyodbc, dot, & graphviz dependencies
sudo apt install -y gnupg \
unixodbc-dev=2.3.9-5 \
graphviz \
libgvc6 \
protobuf-compiler
# We require protoc >= 3.20, but Ubuntu 22.04 - the OS that these Github
# Actions are running as of 2023.05.03 - doesn't have recent versions
# of protoc in its package repository. To work around this, we vendor in
# protoc 3.20.3.
# We can remove the vendored protoc binary and this run step once Github
# Actions moves to a newer version of Ubunutu.
- name: Add vendored `protoc` to $PATH
run: |
echo "./vendor/protoc-3.20.3-linux-x86_64/bin" >> $GITHUB_PATH
# Combine hashes of the Python interpreter, requirements files, and today's
# date into a file whose hash will key the Python virtualenv.
- name: Create Python environment cache key
run: |
md5sum $(which python) > $GITHUB_WORKSPACE/python_cache_key.md5
md5sum lib/min-constraints-gen.txt >> $GITHUB_WORKSPACE/python_cache_key.md5
md5sum lib/test-requirements.txt >> $GITHUB_WORKSPACE/python_cache_key.md5
md5sum lib/setup.py >> $GITHUB_WORKSPACE/python_cache_key.md5
date +%F >> $GITHUB_WORKSPACE/python_cache_key.md5
- name: Restore virtualenv from cache
id: cache-virtualenv
uses: actions/cache@v3
with:
path: venv
key: v1-python-venv-min-${{ hashFiles('**/python_cache_key.md5') }}
- name: Create Virtual Env
run: |
python -m venv venv
source venv/bin/activate
pip install --upgrade pip
deactivate
- name: Activate virtualenv
run: echo 'source venv/bin/activate' >> $HOME/.bash_profile
- name: Generate Protobufs
run: make protobuf
# End of inlined make_init action

AnOctopus marked this conversation as resolved.
Show resolved Hide resolved
- name: Install deps
run: make python-init-test-min-deps
- name: Run Python Tests
run: make pytest
- name: Run Integration Tests
run: make integration-tests
- name: CLI Smoke Tests
run: make cli-smoke-tests
- name: Validate min-constraints-gen
run: |
make gen-min-dep-constraints

git_status=$(git status --porcelain -- lib/min-constraints-gen.txt)
if [[ -n $git_status ]]; then
echo "::error::The min constraints file is out of date! Please run \`make gen-min-dep-constraints\` and commit the result."
echo "::group::git diff lib/min-constraints-gen.txt"
git diff lib/min-constraints-gen.txt
echo "::endgroup::"
exit 1
else
echo "min constraints file is up to date."
fi
13 changes: 12 additions & 1 deletion Makefile
Expand Up @@ -95,9 +95,14 @@ python-init-dev-only:
python-init-test-only: lib/test-requirements.txt
INSTALL_DEV_REQS=false INSTALL_TEST_REQS=true make python-init

.PHONY: python-init-test-min-deps
# Install Streamlit and test requirements, with minimum dependency versions
python-init-test-min-deps:
INSTALL_DEV_REQS=false INSTALL_TEST_REQS=true USE_CONSTRAINTS_FILE=true CONSTRAINTS_URL="lib/min-constraints-gen.txt" make python-init

.PHONY: python-init
python-init:
pip_args=("install" "--editable" "lib[snowflake]");\
pip_args=("--editable" "lib[snowflake]");\
if [ "${USE_CONSTRAINT_FILE}" = "true" ] ; then\
pip_args+=(--constraint "${CONSTRAINTS_URL}"); \
fi;\
Expand Down Expand Up @@ -347,6 +352,12 @@ headers:
pre-commit run insert-license --all-files --hook-stage manual
pre-commit run license-headers --all-files --hook-stage manual

.PHONY: gen-min-dep-constraints
# Write the minimum versions of our dependencies to a constraints file.
gen-min-dep-constraints:
make develop >/dev/null
python scripts/get_min_versions.py >lib/min-constraints-gen.txt

.PHONY: build-test-env
# Build docker image that mirrors circleci
build-test-env:
Expand Down
27 changes: 17 additions & 10 deletions e2e/scripts/st_arrow_altair_chart.py
Expand Up @@ -57,16 +57,23 @@
st.write("Bar chart with overwritten theme props:")
st._arrow_altair_chart(chart.configure_mark(color="black"), theme="streamlit")

source = pd.DataFrame({"category": [1, 2, 3, 4, 5, 6], "value": [4, 6, 10, 3, 7, 8]})
# mark_arc was added in 4.2, but we have to support altair 4.0-4.1, so we
# have to skip this part of the test when testing min versions.
major, minor, patch = alt.__version__.split(".")
if not (major == "4" and minor < "2"):

chart = (
alt.Chart(source)
.mark_arc(innerRadius=50)
.encode(
theta=alt.Theta(field="value", type="quantitative"),
color=alt.Color(field="category", type="nominal"),
source = pd.DataFrame(
{"category": [1, 2, 3, 4, 5, 6], "value": [4, 6, 10, 3, 7, 8]}
)
)

st.write("Pie Chart with more than 4 Legend items")
st._arrow_altair_chart(chart, theme="streamlit")
chart = (
alt.Chart(source)
.mark_arc(innerRadius=50)
.encode(
theta=alt.Theta(field="value", type="quantitative"),
color=alt.Color(field="category", type="nominal"),
)
)

st.write("Pie Chart with more than 4 Legend items")
st._arrow_altair_chart(chart, theme="streamlit")
24 changes: 24 additions & 0 deletions lib/min-constraints-gen.txt
@@ -0,0 +1,24 @@
altair==4.0
blinker==1.0.0
cachetools==4.0
click==7.0
gitpython==3.0.7
importlib-metadata==1.4
numpy==1.19.3
packaging==16.8
pandas==1.3.0
pillow==7.1.0
protobuf==3.20
pyarrow==6.0
pydeck==0.8
pympler==0.9
python-dateutil==2.7.3
requests==2.18
rich==10.14.0
tenacity==8.0.0
toml==0.10.1
tornado==6.0.3
typing-extensions==4.1.0
tzlocal==1.1
validators==0.2
watchdog==2.1.5
28 changes: 15 additions & 13 deletions lib/setup.py
Expand Up @@ -28,6 +28,7 @@
# IMPORTANT: We should try very hard *not* to add dependencies to Streamlit.
# And if you do add one, make the required version as general as possible:
# - Include relevant lower bound for any features we use from our dependencies
# - Always include the lower bound as >= VERSION, to keep testing min versions easy
# - And include an upper bound that's < NEXT_MAJOR_VERSION
INSTALL_REQUIRES = [
"altair>=4.0, <6",
Expand All @@ -36,10 +37,11 @@
"click>=7.0, <9",
# 1.4 introduced the functionality found in python 3.8's importlib.metadata module
"importlib-metadata>=1.4, <7",
"numpy>=1, <2",
"packaging>=14.1, <24",
"pandas>=0.25, <3",
"pillow>=6.2.0, <10",
"numpy>=1.19.3, <2",
"packaging>=16.8, <24",
# Lowest version with available wheel for 3.7 + amd64 + linux
"pandas>=1.3.0, <3",
"pillow>=7.1.0, <10",
# Python protobuf 4.21 (the first 4.x version) is compatible with protobufs
# generated from `protoc` >= 3.20. (`protoc` is installed separately from the Python
# protobuf package, so this pin doesn't actually enforce a `protoc` minimum version.
Expand All @@ -48,28 +50,28 @@
# pyarrow is not semantically versioned, gets new major versions frequently, and
# doesn't tend to break the API on major version upgrades, so we don't put an
# upper bound on it.
"pyarrow>=4.0",
"pyarrow>=6.0",
"pympler>=0.9, <2",
"python-dateutil>=2, <3",
"requests>=2.4, <3",
"rich>=10.11.0, <14",
"python-dateutil>=2.7.3, <3",
"requests>=2.18, <3",
"rich>=10.14.0, <14",
"tenacity>=8.0.0, <9",
"toml<2",
"typing-extensions>=4.0.1, <5",
"toml>=0.10.1, <2",
"typing-extensions>=4.1.0, <5",
"tzlocal>=1.1, <5",
"validators>=0.2, <1",
# Don't require watchdog on MacOS, since it'll fail without xcode tools.
# Without watchdog, we fallback to a polling file watcher to check for app changes.
"watchdog; platform_system != 'Darwin'",
"watchdog>=2.1.5; platform_system != 'Darwin'",
]

# We want to exclude some dependencies in our internal Snowpark conda distribution of
# Streamlit. These dependencies will be installed normally for both regular conda builds
# and PyPI builds (that is, for people installing streamlit using either
# `pip install streamlit` or `conda install -c conda-forge streamlit`)
SNOWPARK_CONDA_EXCLUDED_DEPENDENCIES = [
"gitpython>=3, <4, !=3.1.19",
"pydeck>=0.1.dev5, <1",
"gitpython>=3.0.7, <4, !=3.1.19",
"pydeck>=0.8, <1",
# Tornado 6.0.3 was the current Tornado version when Python 3.8, our earliest supported Python version,
# was released (Oct 14, 2019).
"tornado>=6.0.3, <7",
Expand Down
4 changes: 2 additions & 2 deletions lib/streamlit/testing/element_tree.py
Expand Up @@ -18,7 +18,7 @@
from datetime import date, datetime, time, timedelta
from typing import Any, Generic, List, Sequence, TypeVar, Union, cast, overload

from typing_extensions import Literal, Self, TypeAlias
from typing_extensions import Literal, TypeAlias

from streamlit import util
from streamlit.elements.heading import HeadingProtoTag
Expand Down Expand Up @@ -118,7 +118,7 @@ class Widget(ABC, Element):
key: str | None
_value: Any

def set_value(self, v: Any) -> Self:
def set_value(self, v: Any):
self._value = v
return self

Expand Down
13 changes: 12 additions & 1 deletion lib/test-requirements.txt
Expand Up @@ -10,12 +10,23 @@ opencv-python>=4.5.3
plotly>=5.3.1
pyspark>=3.1.1
pydot>=1.4.2
rich>=11.2.0
scipy>=1.7.3
seaborn>=0.11.2
setuptools>=65.5.1
watchdog>=2.1.5

urllib3>=1.9

hypothesis>=6.17.4
parameterized
pytest
pytest-cov
requests-mock
testfixtures


mypy-protobuf>=3.2

Comment on lines +18 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think the convention here is to alphabetize all dependencies in the list (and also no extra newlines).

Re: the new requirements themselves - it looks like most of these are duplicated from dev-requirements.txt. Should we just be installing dev-requirements.txt for this set of tests, rather than duplicating? Or does that break something with min-dependency testing?

(If the latter, can we add documentation to test-requirements.txt explaining the duplication?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some dev requirements (mainly black) would impose much higher minimum versions of dependencies we share with them, which I wanted to avoid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly some of these should be factored out into a 3rd requirements file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's also possible that we don't mind requiring a more recent typing extensions version, or could roll back to an older version of black that doesn't need something so recent (don't remember why we're on the version we have).

# These requirements exist only for `@st.cache` tests. st.cache is deprecated, and
# we're not going to update its associated tests anymore. Please don't modify
# these pinned versions!
Expand Down
1 change: 1 addition & 0 deletions scripts/check_license_headers.py
Expand Up @@ -49,6 +49,7 @@
r"|^frontend/(\.dockerignore|\.eslintrc|\.prettierignore)$"
r"|^lib/(\.coveragerc|\.dockerignore|MANIFEST\.in|mypy\.ini|pytest\.ini)$"
r"|^lib/(test|dev)-requirements\.txt$"
r"|^lib/min-constraints-gen\.txt"
r"|\.isort\.cfg$"
r"|\.credentials/\.gitignore$"
# Excluding test files, because adding headers may cause tests to fail.
Expand Down