1
0
mirror of https://github.com/certbot/certbot.git synced 2025-08-08 04:02:10 +03:00

remove CERTBOT_NO_PIN (#9634)

Adrien and I added this is in https://github.com/certbot/certbot/pull/6590 in response to https://github.com/certbot/certbot/issues/6582 which I wrote. I now personally think these tests are way more trouble than they're worth.

In almost all cases, the versions pinned in `tools/requirements.txt` are used. The two exceptions to this that come to mind are users using OS packages and pip. In the former, the version of our dependencies is picked by the OS and do not change much on most systems. As for pip, [we only "support it on a best effort basis"](https://eff-certbot.readthedocs.io/en/stable/install.html#alternative-2-pip).

Even for pip users, I'm not convinced this buys us much other than frequent test failures. We have our tests configured to error on all Python warnings and [we regularly update `tools/requirements.txt`](https://github.com/certbot/certbot/commits/master/tools/requirements.txt). Due to that, assuming our dependencies follow normal conventions, we should have a chance to fix things in response to planned API changes long before they make their way to our users. I do not think it is necessary for our tests to break immediately after an API is deprecated.

I think almost all other failures due to these tests are caused by upstream bugs. In my experience, almost all of them will sort themselves out pretty quickly. I think that responding to those that are not or planned API changes we somehow missed can be addressed when `tools/requirements.txt` is updated or when someone opens an issue. I personally don't think blocking releases or causing our nightly tests to fail is at all worth it here. I think removing this frequent cause of test failures makes things just a little bit easier for Certbot devs without costing us much of anything.
This commit is contained in:
Brad Warren
2023-03-27 17:01:27 -07:00
committed by GitHub
parent f004383582
commit 208ef4eb94
6 changed files with 22 additions and 49 deletions

View File

@@ -17,10 +17,6 @@ jobs:
linux-py310:
PYTHON_VERSION: 3.10
TOXENV: py310
linux-py37-nopin:
PYTHON_VERSION: 3.7
TOXENV: py37
CERTBOT_NO_PIN: 1
linux-boulder-v2-integration-certbot-oldest:
PYTHON_VERSION: 3.7
TOXENV: integration-certbot-oldest

View File

@@ -28,17 +28,10 @@ steps:
inputs:
versionSpec: $(PYTHON_VERSION)
addToPath: true
# tools/pip_install.py is used to pin packages to a known working version
# except in tests where the environment variable CERTBOT_NO_PIN is set.
# virtualenv is listed here explicitly to make sure it is upgraded when
# CERTBOT_NO_PIN is set to work around failures we've seen when using an older
# version of virtualenv. The option "-I" is set so when CERTBOT_NO_PIN is also
# set, pip updates dependencies it thinks are already satisfied to avoid some
# problems with its lack of real dependency resolution.
- bash: |
set -e
python3 tools/pipstrap.py
python3 tools/pip_install.py -I tox virtualenv
python3 tools/pip_install.py tox
displayName: Install runtime dependencies
- task: DownloadSecureFile@1
name: testFarmPem

View File

@@ -585,27 +585,20 @@ include our snaps, Docker images, Windows installer, CI, and our development
environments.
In most cases, the file where dependency versions are specified is
``tools/requirements.txt``. There are two exceptions to this. The first is our
"oldest" tests where ``tools/oldest_constraints.txt`` is used instead. The
purpose of the "oldest" tests is to ensure Certbot continues to work with the
oldest versions of our dependencies which we claim to support. The oldest
versions of the dependencies we support should also be declared in our setup.py
files to communicate this information to our users.
The second exception to using ``tools/requirements.txt`` is in our unpinned
tests. As of writing this, there is one test we run nightly in CI where we
leave Certbot's dependencies unpinned. The thinking behind this test is to help
us learn about breaking changes in our dependencies so that we can respond
accordingly.
``tools/requirements.txt``. The one exception to this is our "oldest" tests
where ``tools/oldest_constraints.txt`` is used instead. The purpose of the
"oldest" tests is to ensure Certbot continues to work with the oldest versions
of our dependencies which we claim to support. The oldest versions of the
dependencies we support should also be declared in our setup.py files to
communicate this information to our users.
The choices of whether Certbot's dependencies are pinned and what file is used
if they are should be automatically handled for you most of the time by
Certbot's tooling. The way it works though is ``tools/pip_install.py`` (which
many of our other tools build on) checks for the presence of environment
variables. If ``CERTBOT_NO_PIN`` is set to 1, Certbot's dependencies will not
be pinned. If that variable is not set and ``CERTBOT_OLDEST`` is set to 1,
``tools/oldest_constraints.txt`` will be used as constraints for ``pip``.
Otherwise, ``tools/requirements.txt`` is used as constraints.
variables. If ``CERTBOT_OLDEST`` is set to 1, ``tools/oldest_constraints.txt``
will be used as constraints for ``pip``, otherwise, ``tools/requirements.txt``
is used as constraints.
Updating dependency versions
----------------------------

View File

@@ -1,10 +1,9 @@
#!/usr/bin/env python
# pip installs the requested packages in editable mode and runs unit tests on
# them. Each package is installed and tested in the order they are provided
# before the script moves on to the next package. If CERTBOT_NO_PIN is set not
# set to 1, packages are installed using pinned versions of all of our
# dependencies. See pip_install.py for more information on the versions pinned
# to.
# before the script moves on to the next package. Packages are installed using
# pinned versions of all of our dependencies. See pip_install.py for more
# information on the versions pinned to.
import os
import re
import subprocess

View File

@@ -36,24 +36,18 @@ def main(args):
tools_path = find_tools_path()
with tempfile.TemporaryDirectory() as working_dir:
if os.environ.get('CERTBOT_NO_PIN') == '1':
# With unpinned dependencies, there is no constraint
pip_install_with_print(' '.join(args))
repo_path = os.path.dirname(tools_path)
if os.environ.get('CERTBOT_OLDEST') == '1':
constraints_path = os.path.normpath(os.path.join(
repo_path, 'tools', 'oldest_constraints.txt'))
else:
# Otherwise, we pick the constraints file based on the environment
# variable CERTBOT_OLDEST.
repo_path = os.path.dirname(tools_path)
if os.environ.get('CERTBOT_OLDEST') == '1':
constraints_path = os.path.normpath(os.path.join(
repo_path, 'tools', 'oldest_constraints.txt'))
else:
constraints_path = os.path.normpath(os.path.join(
repo_path, 'tools', 'requirements.txt'))
constraints_path = os.path.normpath(os.path.join(
repo_path, 'tools', 'requirements.txt'))
env = os.environ.copy()
env["PIP_CONSTRAINT"] = constraints_path
env = os.environ.copy()
env["PIP_CONSTRAINT"] = constraints_path
pip_install_with_print(' '.join(args), env=env)
pip_install_with_print(' '.join(args), env=env)
if __name__ == '__main__':

View File

@@ -23,8 +23,6 @@ all_packages = {[base]win_all_packages} certbot-apache
source_paths = acme/acme certbot/certbot certbot-apache/certbot_apache certbot-ci/certbot_integration_tests certbot-ci/snap_integration_tests certbot-ci/windows_installer_integration_tests certbot-compatibility-test/certbot_compatibility_test certbot-dns-cloudflare/certbot_dns_cloudflare certbot-dns-digitalocean/certbot_dns_digitalocean certbot-dns-dnsimple/certbot_dns_dnsimple certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy certbot-dns-gehirn/certbot_dns_gehirn certbot-dns-google/certbot_dns_google certbot-dns-linode/certbot_dns_linode certbot-dns-luadns/certbot_dns_luadns certbot-dns-nsone/certbot_dns_nsone certbot-dns-ovh/certbot_dns_ovh certbot-dns-rfc2136/certbot_dns_rfc2136 certbot-dns-route53/certbot_dns_route53 certbot-dns-sakuracloud/certbot_dns_sakuracloud certbot-nginx/certbot_nginx
[testenv]
passenv =
CERTBOT_NO_PIN
platform =
win: win32
posix: ^(?!.*win32).*$