chore: remove the upper restrictions on the websockets dependency

[tonyandrewmeyer]

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 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.

[jujubot]

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

[jujubot]

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

[cderici]

Yeah testing with those lines removed (and replaced by just websockets) with py38, py39, py310, and py311 worked well for me (they all seem to like websockets-12.0), so I think we can go ahead and get rid of this matrix.

Note that this is in setup.py, so testing with tox (like in the CI) doesn't make sense (as tox doesn't have that restriction)(you can check the current integration test run, it seems to always pick the 12.0 anyways), anyway if you like to test this locally, you can go ahead and run python 3.x pip install . and either check the stdout or /.local/lib/python3.x/site-packages/websockets***.

I'd be happy to run more tests after the PR is updated to make sure nothing else blows up (with ops etc), then we can land it 👍

thanks for working on this! <3

[tonyandrewmeyer]

Note that this is in setup.py, so testing with tox (like in the CI) doesn't make sense (as tox doesn't have that restriction)

Ah, thanks, I missed that tox.ini had dependencies listed separately. I re-checked running pytest outside of tox and both 3.8 and 3.11 pass (as you said, websockets 12 seems to always get picked, which makes sense since it's the latest and still supports 3.8).

I'd be happy to run more tests after the PR is updated to make sure nothing else blows up (with ops etc), then we can land it 👍

Thanks! Let me know if there's anything more I should do.

[cderici]

/build

[cderici]

/merge