Skip to content

Align pre-commit, lint and changelog tooling with Salt - #623

Open
ggiesen wants to merge 17 commits into
saltstack:developfrom
ggiesen:salt-tooling
Open

ggiesen wants to merge 17 commits into
saltstack:developfrom
ggiesen:salt-tooling

Conversation

@ggiesen

@ggiesen ggiesen commented Nov 5, 2026 •

Copy link
Copy Markdown

What does this PR do?

Depends on #621; only the last 7 commits are new here.

Brings the tooling in line with Salt's master branch, as the second step of #622.

  • pre-commit hooks at the same versions and arguments as Salt: pyupgrade --py310-plus, isort and black (default line length) instead of reorder-python-imports and black -l 100, bandit, yamllint, and pylint with saltpylint using a copy of Salt's .pylintrc. pylint replaces flake8, so the flake8 workflow and nox session are gone.
  • The saltpylint blacklist plugin is not loaded: it requires salt.utils.files.fopen and tests.support.mock, and pepper does not depend on salt.
  • The mechanical reformat is its own commit, separate from the hand fixes for pylint findings.
  • Remove the remaining Python 2 fallbacks. The JSONDecodeError fallback looked up json.decode, which does not exist, so it always used ValueError.
  • Add towncrier with Salt's configuration and template, CHANGELOG.md and changelog/, and document the hooks, fragments and the develop branch in the README (it still described a master branch).

What issues does this PR fix or reference?

Fixes #206

Previous Behavior

Hooks did not match Salt, flake8 was the only linter, and there was no changelog.

New Behavior

pre-commit run --all-files runs the same formatters and linters as Salt, and changes carry changelog/<issue>.<type>.md fragments.

Fork CI: https://github.com/ggiesen/pepper/actions/runs/42174288007

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated: Existing tests pass; no behavior changes beyond the pylint fixes listed in that commit

Commits signed with GPG?

No

Pepper gives SALTAPI_* environment variables precedence over the config
file, and the test suite never cleared them, despite the pepper_cli
fixture claiming to run "inside an empty env". A developer with
SALTAPI_URL, SALTAPI_USER or SALTAPI_EAUTH exported would have every CLI
test log in to that salt-api with the test credentials instead of the
salt-api started by the suite, and unit tests such as
test_interactive_logins fail.

Add an autouse fixture that removes SALTAPI_*, PEPPER* and MASTER_CONFIG
from the environment for every test.
The imp module was removed in Python 3.12, so collecting
tests/unit/test_init.py fails there. Load the fresh copy of
pepper/__init__.py with importlib.util instead.

The version is read from the installed package metadata rather than
setup.py, so stop renaming setup.py during the test (which left the
checkout broken if the test was interrupted), compare against
importlib.metadata instead of setuptools_scm, and cover the
not-installed fallback explicitly.
- importlib-metadata<5.0.0 cannot be co-installed with current Salt
  releases and pepper uses the standard library importlib.metadata.
- The cryptography and pyzmq pins conflict with what Salt itself
  requires (cryptography>=48 for 3006.30 on Python 3.10+); let Salt
  choose them.
- mock and setuptools_scm are no longer used by the test suite.
- Bump pytest-salt-factories to 1.0.5.
[os.environ.get("X")] or [...] is always a one element list, so without
API_BACKEND and PYTHON_VERSIONS set the tests session ran with
python=None and --salt-api-backend=rest_None. Split the environment
values and fall back to the full lists when they are unset, moving the
default Python versions to 3.10-3.13.

Also make pip quiet outside CI as the comment intends (CI_RUN is a
string or None, never False) and replace the coverage 5.5 pin, which
predates Python 3.12 support.
Test Python 3.10 through 3.13 against the current Salt releases
(3006.30, 3007.17, 3008.5) and master, dropping Python 3.8 and 3.9,
which are end of life.

Pin actions/checkout and actions/setup-python to the same commit SHAs
the Salt repository uses and restrict the workflow token to read-only
contents access.
CI runs on GitHub Actions. The Travis and Azure Pipelines definitions
target Python 2.7 to 3.8 and Salt 2018.3/2019.2, the Makefile drives the
removed tox setup, and nothing uploads coverage to Codecov any more.
azure-pipelines.yml also contained a Codecov upload token.

Replace the Travis and Codecov badges in the README with the GitHub
Actions status badge.
The test minion never completes authentication with the master for
Salt 3006 on Python 3.12 and 3.13, or Salt 3007 on Python 3.13, so
every test that needs a minion errors during setup.

Salt 3007 on Python 3.12 usually passes, but in 7 of 50 recent CI jobs
the minion failed to start, stopped answering, or took about a minute
per call. Five of those jobs failed and the slowest ran for 81 minutes,
against 2 to 7 minutes normally, so exclude it as well.

Salt 3006 and 3007 declare support up to Python 3.10; both pass on
3.11, so keep those.
On Salt 3008 the netapi ssh client authorizes the request itself and
looks the token up in salt.cache, which does not find the token minted
through /login or /token, so salt-api answers 401 for both the session
and /run paths. Every other client still passes on 3008. Mark the test
xfail (non-strict) until the cause is understood instead of letting it
hold the rest of the matrix red.
A job normally finishes in 2 to 7 minutes. When the test minion hangs,
the job otherwise runs until GitHub's 6 hour limit and holds a runner
the whole time.
test_token wrote a real ~/.peppercache, and a developer's ~/.pepperrc
could leak into the tests. Point HOME (and USERPROFILE) at an empty
directory for every test.
Use the same hook versions and arguments as Salt's master branch:
pyupgrade --py310-plus, isort and black at Salt's settings instead of
reorder-python-imports and black -l 100, bandit, yamllint and pylint
with saltpylint using a copy of Salt's .pylintrc.

The saltpylint blacklist plugin is not loaded. It requires Salt's own
helpers (salt.utils.files.fopen, tests.support.mock), and pepper does
not depend on salt.

pylint replaces flake8, so the flake8 workflow and nox session are
removed and the pre-commit job in the test workflow is the only lint
step.
No functional changes. This is the output of the pre-commit formatting
hooks added in the previous commit.
Drop the urllib2, ConfigParser, raw_input and logging.NullHandler
fallbacks. The JSONDecodeError fallback looked up json.decode, which
does not exist, so it always fell back to ValueError; import
json.JSONDecodeError instead.
- Remove unreachable return statements after raise in req_get and
  req_stream.
- Format the 'Cannot open file' message instead of passing the file
  name as a second exception argument, which printed a tuple.
- Set the umask before the try block in login() so the finally clause
  cannot reference an unset variable.
- Open text files as UTF-8 instead of the locale encoding.
- Mark the intentional catch-all handlers and the optparse import.

The broad handlers around the token cache and the global SSL context
are left in place; later changes replace that code.
Use the same towncrier configuration and template as Salt, with
fragments named changelog/<issue>.<type>.md. Document the pre-commit
hooks, changelog fragments and the develop branch in the README; the
master branch it mentioned no longer exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add changelog

1 participant