dalibo / check_patroni

A nagios plugin for patroni.
PostgreSQL License
7 stars 3 forks source link

Release V2.0.0 #69

Closed blogh closed 5 months ago

blogh commented 6 months ago

Hi @dlax,

I am a little lost by this error.

Does this mean I should drop 3.7, limit the version of requests ?

https://github.com/dalibo/check_patroni/actions/runs/8617904548/job/23619014594?pr=69#step:6:33

 .tox/mypy/lib/python3.12/site-packages/requests-stubs/structures.pyi:21: error: Positional-only parameters are only supported in Python 3.8 and greater  [syntax]
dlax commented 6 months ago

I suspect there's something incompatible with mypy being locked to an old version in: https://github.com/dalibo/check_patroni/blob/e0589b97a8f387a69bf3d683f10f4049f995b165/tox.ini#L26 and the type definitions of requests, provided by typeshed, that have probably changed since that version.

In general, blocking a dependency like that is not a good idea, because, as you can see now, it just shadows issues that would quite likely resurge later on, quite often when you will have little time to handle the original issue and even probably forgotten what the original issue was about...

dlax commented 6 months ago

So I'd suggest to remove the constraint on mypy version and fix or type: ignore issues.

blogh commented 6 months ago

So I'd suggest to remove the constraint on mypy version and fix or type: ignore issues.

I don't understand why I don't have the same errors on my computer.

(.venv_312) benoit@benoit-dalibo:~/git/dalibo/check_patroni$ tox -e lint,mypy
lint: commands[0]> codespell /home/benoit/git/dalibo/check_patroni/check_patroni /home/benoit/git/dalibo/check_patroni/tests /home/benoit/git/dalibo/check_patroni/docs/ /home/benoit/git/dalibo/check_patroni/RELEASE.md /home/benoit/git/dalibo/check_patroni/CONTRIBUTING.md
lint: commands[1]> black --check --diff /home/benoit/git/dalibo/check_patroni/check_patroni /home/benoit/git/dalibo/check_patroni/tests
All done! ✨ 🍰 ✨
23 files would be left unchanged.
lint: commands[2]> flake8 /home/benoit/git/dalibo/check_patroni/check_patroni /home/benoit/git/dalibo/check_patroni/tests
lint: commands[3]> isort --check --diff /home/benoit/git/dalibo/check_patroni/check_patroni /home/benoit/git/dalibo/check_patroni/tests
lint: OK ✔ in 1.07 seconds
.pkg: _optional_hooks> python /home/benoit/git/dalibo/check_patroni/.venv_312/lib64/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_sdist> python /home/benoit/git/dalibo/check_patroni/.venv_312/lib64/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_wheel> python /home/benoit/git/dalibo/check_patroni/.venv_312/lib64/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: prepare_metadata_for_build_wheel> python /home/benoit/git/dalibo/check_patroni/.venv_312/lib64/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: build_sdist> python /home/benoit/git/dalibo/check_patroni/.venv_312/lib64/python3.8/site-packages/pyproject_api/_backend.py True setuptools.build_meta
mypy: install_package> python -I -m pip install --force-reinstall --no-deps /home/benoit/git/dalibo/check_patroni/.tox/.tmp/package/150/check_patroni-2.0.0.tar.gz
mypy: commands[0]> mypy --install-types --non-interactive
check_patroni/cluster.py:32: error: "Counter" is not subscriptable, use "typing.Counter" instead  [misc]
check_patroni/cluster.py:34: error: "Counter" is not subscriptable, use "typing.Counter" instead  [misc]
Found 2 errors in 1 file (checked 24 source files)
mypy: exit 1 (0.89 seconds) /home/benoit/git/dalibo/check_patroni> mypy --install-types --non-interactive pid=214494
  lint: OK (1.07=setup[0.08]+cmd[0.18,0.37,0.28,0.16] seconds)
  mypy: FAIL code 1 (5.62=setup[4.72]+cmd[0.89] seconds)
  evaluation failed :( (6.83 seconds)
blogh commented 6 months ago

ra never mind, I scratched my venv en retried (wich I thought I already did). It's the same error now. mybad.

dlax commented 6 months ago

About the mypy issue; I don't see it with Python 3.11 locally. So maybe you could run that check with python 3.11 in CI (as you do already for tests)?

blogh commented 5 months ago

ok ... I am just dumb. I didn't see the restriction on the version of mypy in tox.ini and just modified it in requirements-dev.txt.