-
Notifications
You must be signed in to change notification settings - Fork 222
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
bump python versions and use docker slim versions #1661
Conversation
🌐 Coverage report
|
I'll run a few builds in the CI to discard any issues with the full test coverage |
/test full |
for the framework psycopg2-newest
with framework mysqlclient-newest
/test full |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Small suggestion :
# Use --no-cache-dir flag.
# pre-install base requirements
RUN python -m pip install --no-cache-dir --progress-bar off -U pip && pip install --no-cache-dir --progress-bar off -r /reqs-base.txt
@@ -7,7 +7,7 @@ docker_pip_cache="/tmp/cache/pip" | |||
|
|||
cd tests | |||
|
|||
docker build --build-arg PYTHON_IMAGE=python:3.6 -t python-linters . | |||
docker build --build-arg PYTHON_IMAGE=python:3.8 -t python-linters . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if these scripts are still in use. We use pre-commit for black/isort/flake8. The only reference I could find is in run-tests-locally.asciidoc
From what I can tell, we might as well just delete these three scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let me review it in a follow up, as I don't wanna change any further behaviours in this PR but only bump the version to use a slim one and default to a major python version in those scripts.
Thanks @amannocci , this change was not aimed to modify any of the internals but the docker image base image and default python version, for now I'll not make any further changes so this can be merged and in a follow up the implementation can be revisited 🙏 |
…ith-k8s-skaffold * upstream/main: bump python versions and use docker slim versions (elastic#1661) use github actions for pre-commit checks (elastic#1658) Use typing.TypeVar on decorators' type hints (elastic#1655) Use minimum of 5 seconds for interval between central config calls (elastic#1652) fix errors in pymongo tests introduced in elastic#1639 (elastic#1648) only use content-type and content-encoding headers for POSTing events (elastic#1651) fix starlette 0.21 tests (elastic#1653)
What does this pull request do?
Use smaller docker images (slim) and bump the version to use the supported python version for some of the scripts
Why
vs
Then the docker image with all the tooling is about 2x smaller:
vs
Implementation details
There were some missing packages required to run some of the tests, such as:
libmariadb-dev
to fixmariadb_config: not found
when using the frameworkmysqlclient-newest
libpq-dev
to fixpg_config executable not found
when using the frameworkpsycopg2-newest
build-essential
to fixgcc executable not found
when using the frameworktwisted
make
to fix the run of all the tests.gpg
to fix the docker image generation.