Commit 2a5dfb1
authored
Merge pull request #1007 from tonyandrewmeyer/looser-websocket-requirements
#1007
#### Description
With the way that the `websockets` dependency is currently specified, it's impossible to use the same version of the library with Python 3.8, 3.9, and 3.10 - even though the `websockets` library does have many versions that work with all those versions. This means that if a dependency that uses `python-libjuju` is tightly pinned (e.g. a lock file) then different locks are required for each Python version. This can be done, but it's inconvenient, and doesn't seem to be required.
This PR simply removes the upper restriction, so that Python 3.8 can use `websockets` 8.1 and above (the current latest version still supports 3.8), Python 3.9 can use `websockets` 9.0 and above, and Python 3.10+ can use `websockets` 10.0 and above.
#### QA Steps
```
~/code/python-libjuju$ python -m tox
[...]
The HTML pages are in docs/_build.
lint: OK (1.78=setup[1.35]+cmd[0.43] seconds)
py3: FAIL code 1 (25.00=setup[15.00]+cmd[0.89,5.21,3.90] seconds)
py38: OK (4.63=setup[0.16]+cmd[0.68,0.61,3.18] seconds)
py39: OK (4.43=setup[0.01]+cmd[0.60,0.58,3.24] seconds)
py310: OK (4.35=setup[0.01]+cmd[0.57,0.58,3.20] seconds)
py311: OK (4.09=setup[0.01]+cmd[0.61,0.56,2.91] seconds)
docs: OK (52.06=setup[12.09]+cmd[0.02,39.95] seconds)
evaluation failed :( (96.38 seconds)
```
py3 in the above is 3.12 - that's not in the tox list so (if I understand correctly) not yet supported, so the failure is expected (if the issue is related to `websockets` I'm happy to try to address it in this PR too).
#### Notes & Discussion
Generally, this sort of "per Python version" specification should not be required, because the dependency itself should specify the supported Python versions, and `pip` (or whatever dependency resolver you're using) should handle that automatically.
The main branch of `websockets` does have 3.8 specified as the minimum version in pyproject.toml, as does the 12.0 tag, and all the 11.x tags have 3.7 in pyproject.toml. All the 8.x, 9.x, and 10.x have an appropriate (according to the changelog) Python minimum version specified in setup.py. This means that the three lines really shouldn't be necessary and it should be able to be just `websockets>=8.1` (or `websockets>=8.1,<13` to avoid breakage when 13 is released) without any Python version specification. I'm happy to change the PR to that.
It's possible that there were some issues with specific `websockets` version and specific Python version combinations, but reading over the `websockets` changelog there isn't mention of anything that indicates that entire sets would need to be excluded like this.
The [commit](15fe7e3) that introduced this practice says:
> According to the websockets changelog[0]:
> - v8.0 requires python >= 3.6
> - v8.1 adds support for python3.8
> - v9.0 adds support for python3.9
> - v10.0 adds support for python3.10
>
> This change is attempting to cover all these scenarios. The previous
> behavior was allowing some unwanted scenarios, e.g. websockets==9.0 and
> python3.10 combination. Which is incompatible.
I wasn't around at that time, so I'm not sure what was happening (and there's no linked bug/issue) but it seems odd that was required. Even 8.x `websockets` correctly specifies the minimum Python version, so whatever dependency resolver was being used should have automatically handled everything required above.
In any case, it seems like the commit was not meaning to put an upper limit, and that was perhaps accidental?
If there are specific checks I can do to verify that the various `websockets` versions are ok in the various Python versions, please me know and I'm happy to do that.1 file changed
Lines changed: 1 addition & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
29 | | - | |
30 | | - | |
| 28 | + | |
31 | 29 | | |
32 | 30 | | |
33 | 31 | | |
| |||
0 commit comments