Make check-haproxy-stats Python 2.7 / 3.7 dual-compatible - #13
Open
jjpersch wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Updates project packaging/testing metadata to reflect and exercise Python 2.7 + Python 3.7 compatibility (without changing runtime code).
Changes:
- Add
py37totoxenvlist so tests run under Python 3.7 as well as 2.7. - Remove unused
futurefrominstall_requires. - Add the
Programming Language :: Python :: 3.7classifier insetup.py.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tox.ini | Expands tox envlist to include py37 for dual-interpreter test runs. |
| setup.py | Drops unused future dependency and adds a Python 3.7 trove classifier. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Nothing in the package imports `future`. The three entry-point modules use `from __future__ import print_function`, which is the stdlib `__future__` module and needs no third-party package. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The source was already dual-compatible: all three entry-point modules carry `from __future__ import print_function`, and there is no py2-only syntax anywhere in the package. Only the metadata said otherwise. tox.ini and the setup.py classifiers were narrowed to 2.7 in May 2017 (#3 and #5) on the stated grounds that "our dependency is unfortunately python 2.7 only". That does not hold up. The dependency is haproxy-stats, tested here at 1.3; haproxystats/__init__.py is a single module that upstream left untouched from 2016-03-10 until after 1.3 shipped, so it is identical across the whole 1.3 line, and it contains no py2-only constructs. It imports and runs on 3.7. Verified on both interpreters: all 27 tests pass under `python setup.py test` on 2.7.18 and 3.7.17 (one intentional skip, the disabled gmetric case), and all three console scripts run. Only 3.7 is claimed, not a bare `Programming Language :: Python :: 3`, since 3.8+ has not been tested. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
jjpersch
force-pushed
the
jjpersch/py37-dual-compat
branch
from
August 1, 2026 20:13
337f488 to
d7180ec
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tox.ini:2
- Adding
py37to the defaultenvlistmakestoxfail withInterpreterNotFoundon machines/CI images that don’t have an explicit Python 3.7 interpreter installed. If the goal is “run 3.7 when available”, consider enablingskip_missing_interpretersso contributors can still runtoxwithout installing 3.7 locally.
[tox]
envlist = py27, py37, flake8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes this package Python 2.7 / 3.7 dual-compatible. No source code changes were needed: the package was already dual-compatible and only the packaging metadata claimed otherwise. This PR adds the 3.7 tox env and setup.py classifier, and drops the unused
futuredependency.Background
All three entry-point modules already carry
from __future__ import print_function, and there is no py2-only syntax anywhere in the package. The 2.7-only metadata dates to May 2017, when #3 (massage tox list -- our dep is unfortunately 27 only) cutenvlistdown topy27, flake8and #5 removed theProgramming Language :: Python :: 3*classifiers. Both cited the same reason: "one of our dependencies is python 2.7 [only]".That reason does not hold up. The dependency in question is
haproxy-stats.haproxystatsis a single module, and upstream (bcicen/haproxy-stats) lefthaproxystats/__init__.pyuntouched from 2016-03-10 until 2018-03-01 -- so the file is identical across the entire 1.3 line. It contains no py2-only constructs, and it imports and runs on 3.7. The 2017 conclusion appears never to have been tested, and it has kept this package pinned to a long-dead interpreter ever since.futureis a separate matter: nothing in the package imports it. The modules use__future__, which is stdlib and needs no third-party package.What changed
setup.py: droppedfuturefrominstall_requires; nothing imports it.setup.py: added theProgramming Language :: Python :: 3.7classifier alongside 2.7. Deliberately not adding a bare:: 3-- only 3.7 has been verified.tox.ini:envlist = py27, py37, flake8, so both interpreters are exercised rather than just 2.7.skip_missing_interpretersis deliberately left at its default offalse. The point of this change is to prove that both interpreters pass; lettingtoxexit 0 when one of them is absent would quietly turn that back into "whatever happened to be installed passed", which is how the stale 2.7-only claim went unchallenged for as long as it did. Anyone who lacks an interpreter locally can run a single env (tox -e py27) or pass--skip-missing-interpreters, which takes precedence over the ini and so does not weaken the committed config. Note this is not a new constraint:envlistalready required 2.7, which is by now the harder of the two to have on hand.check_haproxy_stats/. The source did not need porting.Verification
Both interpreters were run against
haproxy-stats1.3, installed from the upstream git revision that carries that version:python setup.py test(the exact command the tox testenv runs) on CPython 2.7.18: 27 tests,OK (skipped=1).python setup.py teston CPython 3.7.17: 27 tests,OK (skipped=1).test_report_haproxy_rates, skipped as "gmetric reporting disabled".check-haproxy-stats-up,check-haproxy-stats-5xx,metrics-haproxy-stats-5xx) start and serve--helpon both interpreters, andcheck-haproxy-stats-upwas additionally run against an absent stats endpoint on both to exercise the realhaproxystats/requestsnetwork path; it degrades to the expected error on each.Two pre-existing problems are NOT addressed here, because both are outside the scope of the interpreter work and one is in a module this PR deliberately does not touch:
tox -e flake8is already red on master, independent of this change:F401forsocket,subprocess, and.haproxy_utilinmetrics_haproxy_stats_5xx.py, plus oneE501atcheck_haproxy_stats_up.py:56. The threeF401s are leftovers from 873632e/3a7a0c1, which commented out the gmetric body but left its imports behind.metrics_haproxy_stats_5xx.pyis now entirely commented out --_report_haproxy_ratesjustreturn 0s. The module and itsmetrics-haproxy-stats-5xxconsole script are a deletion candidate, to be handled in their own PR. Deleting them also clears three of the four flake8 findings above.Deployment plan and risks
Low risk. This PR ships no behavior change: the installed Python is identical to 3.7.0, and the only packaging change is one fewer name in
install_requires, for a package that was never imported.Merging this alone deploys nothing. Follow-up, as separate work: cut a release, then update downstream consumers that pin this package by exact version. Consumers that install with
--no-depsmust continue to pinhaproxy-statsexplicitly themselves, sinceinstall_requiresis not resolved in that mode.Declaring 3.7 support does not change which interpreter any existing consumer runs; it only removes this package as a blocker for consumers that want to move.