Switch from `ws4py` to `websockets` #626

hamistao · 2025-01-13T15:13:55Z

This aims to remove ws4py as a required dependency for pylxd (although keeping it as an extra dependency for tests so we can test for backwards compat), and instead use the websockets library.
This addresses the problems reported in #615 and #585

hamistao · 2025-01-14T10:21:10Z

@simondeziel To my surprise, the 3.8 tests ran indefinetely here. But before (potentially) wasting time looking for the cause, I am now thinking if it isn't time to drop support for Python 3.8 and include 3.13 in our tests. 3.8 reached EOL in October 7, 2024.

Feel free to review this either way and I can address either the failing tests or the version update along with your requested changes.

Bumps [pytest-cov](https://github.com/pytest-dev/pytest-cov) from 2.10.1 to 5.0.0. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst">pytest-cov's changelog</a>.</em></p> <blockquote> <h2>5.0.0 (2024-03-24)</h2> <ul> <li>Removed support for xdist rsync (now deprecated). Contributed by Matthias Reichenbach in <code>[canonical#623](pytest-dev/pytest-cov#623) <https://github.com/pytest-dev/pytest-cov/pull/623></code>_.</li> <li>Switched docs theme to Furo.</li> <li>Various legacy Python cleanup and CI improvements. Contributed by Christian Clauss and Hugo van Kemenade in <code>[canonical#630](pytest-dev/pytest-cov#630) <https://github.com/pytest-dev/pytest-cov/pull/630></code><em>, <code>[canonical#631](pytest-dev/pytest-cov#631) <https://github.com/pytest-dev/pytest-cov/pull/631></code></em>, <code>[canonical#632](pytest-dev/pytest-cov#632) <https://github.com/pytest-dev/pytest-cov/pull/632></code>_ and <code>[canonical#633](pytest-dev/pytest-cov#633) <https://github.com/pytest-dev/pytest-cov/pull/633></code>_.</li> <li>Added a <code>pyproject.toml</code> example in the docs. Contributed by Dawn James in <code>[canonical#626](pytest-dev/pytest-cov#626) <https://github.com/pytest-dev/pytest-cov/pull/626></code>_.</li> <li>Modernized project's pre-commit hooks to use ruff. Initial POC contributed by Christian Clauss in <code>[canonical#584](pytest-dev/pytest-cov#584) <https://github.com/pytest-dev/pytest-cov/pull/584></code>_.</li> </ul> <h2>4.1.0 (2023-05-24)</h2> <ul> <li>Updated CI with new Pythons and dependencies.</li> <li>Removed rsyncdir support. This makes pytest-cov compatible with xdist 3.0. Contributed by Sorin Sbarnea in <code>[canonical#558](pytest-dev/pytest-cov#558) <https://github.com/pytest-dev/pytest-cov/pull/558></code>_.</li> <li>Optimized summary generation to not be performed if no reporting is active (for example, when <code>--cov-report=''</code> is used without <code>--cov-fail-under</code>). Contributed by Jonathan Stewmon in <code>[canonical#589](pytest-dev/pytest-cov#589) <https://github.com/pytest-dev/pytest-cov/pull/589></code>_.</li> <li>Added support for JSON reporting. Contributed by Matthew Gamble in <code>[canonical#582](pytest-dev/pytest-cov#582) <https://github.com/pytest-dev/pytest-cov/pull/582></code>_.</li> <li>Refactored code to use f-strings. Contributed by Mark Mayo in <code>[canonical#572](pytest-dev/pytest-cov#572) <https://github.com/pytest-dev/pytest-cov/pull/572></code>_.</li> <li>Fixed a skip in the test suite for some old xdist. Contributed by a bunch of people in <code>[canonical#565](pytest-dev/pytest-cov#565) <https://github.com/pytest-dev/pytest-cov/pull/565></code>_.</li> </ul> <h2>4.0.0 (2022-09-28)</h2> <p><strong>Note that this release drops support for multiprocessing.</strong></p> <ul> <li> <p><code>--cov-fail-under</code> no longer causes <code>pytest --collect-only</code> to fail Contributed by Zac Hatfield-Dodds in <code>[canonical#511](pytest-dev/pytest-cov#511) <https://github.com/pytest-dev/pytest-cov/pull/511></code>_.</p> </li> <li> <p>Dropped support for multiprocessing (mostly because <code>issue 82408 <https://github.com/python/cpython/issues/82408></code>_). This feature was mostly working but very broken in certain scenarios and made the test suite very flaky and slow.</p> <p>There is builtin multiprocessing support in coverage and you can migrate to that. All you need is this in your <code>.coveragerc</code>::</p> <p>[run] concurrency = multiprocessing</p> </li> </ul>  </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/5295ce01c84262cec88f31255e9ac538718f3047"><code>5295ce0</code></a> Bump version: 4.1.0 → 5.0.0</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/1181b067972bf94569f8011f3b18f271690f9ab1"><code>1181b06</code></a> Update changelog.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/9757222e2e044361e70125ebdd96e5eb87395983"><code>9757222</code></a> Fix a minor grammar error (<a href="https://redirect.github.com/pytest-dev/pytest-cov/issues/636">#636</a>)</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/9f5cd81a0dbe3fe41681efdbef516c08988fe8ff"><code>9f5cd81</code></a> Cleanup releasing instructions. Closes <a href="https://redirect.github.com/pytest-dev/pytest-cov/issues/616">#616</a>.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/93b5047ec5050d63c10a6fe16a09b671a7a03df8"><code>93b5047</code></a> Add test for pyproject.toml loading without explicit --cov-config. Ref <a href="https://redirect.github.com/pytest-dev/pytest-cov/issues/508">#508</a>.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/ff50860d7c67b920503745d92a3f0944cf41f982"><code>ff50860</code></a> docs: add config instructions for pyproject.toml.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/4a5a4b5fa4b1c63ddcab5cbc1813798c9b6f1d36"><code>4a5a4b5</code></a> Keep GitHub Actions up to date with GitHub's Dependabot</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/1d7f55963d5138f41c452a946f7cca7e0b6ee8b2"><code>1d7f559</code></a> Fix or remove URLs that are causing docs tests to fail</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/6a5af8e85b8242ac815f33e26adf9068f5f0ebc3"><code>6a5af8e</code></a> Update changelog.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/d9fe8dfed15023d3410dd299c5092e755b8981c2"><code>d9fe8df</code></a> Switch to furo. Closes <a href="https://redirect.github.com/pytest-dev/pytest-cov/issues/618">#618</a>.</li> <li>Additional commits viewable in <a href="https://github.com/pytest-dev/pytest-cov/compare/v2.10.1...v5.0.0">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pytest-cov&package-manager=pip&previous-version=2.10.1&new-version=5.0.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>

codecov · 2025-01-19T21:04:42Z

Codecov Report

Attention: Patch coverage is 26.58228% with 58 lines in your changes missing coverage. Please review.

Project coverage is 90.70%. Comparing base (42332f8) to head (a07dd09).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
pylxd/client.py	16.27%	36 Missing ⚠️
pylxd/models/instance.py	12.00%	22 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #626      +/-   ##
==========================================
- Coverage   95.44%   90.70%   -4.74%     
==========================================
  Files          32       32              
  Lines        3224     3272      +48     
==========================================
- Hits         3077     2968     -109     
- Misses        147      304     +157

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dimaqq · 2025-03-27T12:12:54Z

pylxd/client.py



-class _WebsocketClient(WebSocketBaseClient):
+class _WebsocketClient(ClientConnection):


Uninformed opinion:
Personally I'd prefer composition over inheritance.

I think conceptually it makes more sense to use inheritance here because the object _WebsocketClient represents a client, not something that includes the logic of a client. It has the capabilities of a client, no more, no less.

Then again this is somewhat open to interpretation. Also, this fits better into the standards we set throughout pylxd code.

dimaqq · 2025-03-27T12:19:17Z

pylxd/client.py

+            # PROTOCOL_TLS_CLIENT does not exist on older Python versions
+            protocol = getattr(ssl, "PROTOCOL_TLS_CLIENT", ssl.PROTOCOL_TLSv1_2)


I've dug into this recently in another project and my conclusion was that PROTOCOL_TLS_CLIENT is in fact available in Python 3.8, which is the oldest Python version supported by this library.

https://docs.python.org/3/library/ssl.html#ssl.PROTOCOL_TLS_CLIENT states that that's available from 3.6

the default value in create_default_context() became PROTOCOL_TLS_CLIENT in Python 3.10, but the constant was available long before that.

meanwhile PROTOCOL_TLSv1_2 has been deprecated, although not removed yet.

Thanks for this! I guess we started using this pattern back when we supported older python versions and failed to keep track on whether the ones we support now have it.

The rationals for dropping support for Python 3.8 are: * this used to be the default Python version in Ubuntu 20.04 which is now out of standard support (EOL) * Ubuntu releases (including 20.04 now in ESM) all use the ancient pylxd version 2.2.10 (lacking VM support) so they won't be affected * Supporting Python 3.8 is holding back development (#615 (comment)) If accepted, this will unblock #626 If anyone using Ubuntu 20.04 with ESM still wants to run `pylxd` from git, it's possible to install Python 3.9 on 20.04/Focal: ``` $ rmadison python3.9 python3.9 | 3.9.5-3ubuntu0~20.04.1 | focal-security/universe | source, amd64, arm64, armhf, i386, ppc64el, riscv64, s390x python3.9 | 3.9.5-3ubuntu0~20.04.1 | focal-updates/universe | source, amd64, arm64, armhf, i386, ppc64el, riscv64, s390x ```

Signed-off-by: hamistao <[email protected]>

This function is responsible for creating a `websockets.sync.ClientConnection` client object. Signed-off-by: hamistao <[email protected]>

This handles the new default `_WebsocketClient` appropriately and accepts a custom `ClientConnection` as an argument. If provided client is a `ws4py` client instead, fallback on handling it as such for backwards compatibility. Signed-off-by: hamistao <[email protected]>

Now the default events websocket client connects to the server on initialization, so integration tests make more sense and we shall add them to substitute the unit tests Signed-off-by: hamistao <[email protected]>

Signed-off-by: hamistao <[email protected]>

…ckets` Signed-off-by: hamistao <[email protected]>

Making `payload` as part of `kwargs` is important to use payload as an additional argument in `create_client_connection`. Also I took the liberty of improving the old log message. Signed-off-by: hamistao <[email protected]>

This creates the three clients (for stdin, stdout and stderr) using `create_client_connection`, same as in `events`. And creates a thread for each one using the module `threading`. The logic for closing the output sockets is kept similar to how it was. Signed-off-by: hamistao <[email protected]>

Now `create_client_connection` connects to the server on client initialization so these tests are now broken. Signed-off-by: hamistao <[email protected]>

Signed-off-by: hamistao <[email protected]>

This keeps `ws4py` as an optional dependency for tests, since we use it to tests backwards compatibility with user provided `ws4py` client objects. Signed-off-by: hamistao <[email protected]>

Signed-off-by: Simon Deziel <[email protected]>

simondeziel · 2025-06-27T21:18:16Z

I'll close this PR as it breaks all the exec and events tests (disabled by the last 2 commits to confirm).

auvipy · 2025-06-28T03:19:39Z

ws4 is now maintained again. and got a new release. Also a new maintainer here

simondeziel · 2025-06-28T20:21:58Z

ws4 is now maintained again. and got a new release. Also a new maintainer here

Thank you @auvipy that's reassuring. Thanks for doing this often thankless job!

hamistao force-pushed the update_weblocket_lib branch 10 times, most recently from 064c6f2 to 1d9eebc Compare January 14, 2025 09:56

hamistao force-pushed the update_weblocket_lib branch from 1d9eebc to 0183a6c Compare January 14, 2025 10:38

hamistao marked this pull request as ready for review January 14, 2025 10:40

hamistao force-pushed the update_weblocket_lib branch from 0183a6c to 579d064 Compare January 14, 2025 14:31

hamistao force-pushed the update_weblocket_lib branch 4 times, most recently from 6d6431c to a07dd09 Compare January 19, 2025 20:53

simondeziel mentioned this pull request Feb 17, 2025

What should I do about ws4py? #615

Closed

dimaqq reviewed Mar 27, 2025

View reviewed changes

hamistao force-pushed the update_weblocket_lib branch from a07dd09 to 22a72a7 Compare March 27, 2025 22:10

simondeziel mentioned this pull request Jun 9, 2025

Drop support for Python 3.8 #646

Merged

simondeziel self-assigned this Jun 9, 2025

simondeziel force-pushed the update_weblocket_lib branch from 22a72a7 to c6c9589 Compare June 27, 2025 18:00

hamistao added 3 commits June 27, 2025 17:12

pylxd/client: Update _WebsocketClient to use websockets

5378867

Signed-off-by: hamistao <[email protected]>

pylxd/client: Create create_client_connection

9ee42e8

This function is responsible for creating a `websockets.sync.ClientConnection` client object. Signed-off-by: hamistao <[email protected]>

hamistao and others added 10 commits June 27, 2025 17:12

pylxd/tests: Skip unit tests for .events

48c3b75

Now the default events websocket client connects to the server on initialization, so integration tests make more sense and we shall add them to substitute the unit tests Signed-off-by: hamistao <[email protected]>

integration: Add tests for events

b890c5d

Signed-off-by: hamistao <[email protected]>

pylxd/models/instance: Update _CommandWebsocketClient to use `webso…

7978b78

…ckets` Signed-off-by: hamistao <[email protected]>

pylxd/tests: Skip unit tests for .execute

4eaeb6e

Now `create_client_connection` connects to the server on client initialization so these tests are now broken. Signed-off-by: hamistao <[email protected]>

doc/source: Update Events docs

73c365a

Signed-off-by: hamistao <[email protected]>

setup: Switch ws4py for websockets

fa61426

This keeps `ws4py` as an optional dependency for tests, since we use it to tests backwards compatibility with user provided `ws4py` client objects. Signed-off-by: hamistao <[email protected]>

integration/test_client: skip events tests (broken with websockets)

6c6c1d4

Signed-off-by: Simon Deziel <[email protected]>

integration/test_containers: skip exec tests (broken with websockets)

a6e6326

Signed-off-by: Simon Deziel <[email protected]>

simondeziel force-pushed the update_weblocket_lib branch from c6c9589 to a6e6326 Compare June 27, 2025 21:12

simondeziel closed this Jun 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Switch from `ws4py` to `websockets` #626

Switch from `ws4py` to `websockets` #626

Uh oh!

hamistao commented Jan 13, 2025

Uh oh!

hamistao commented Jan 14, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jan 19, 2025

Uh oh!

dimaqq Mar 27, 2025

Uh oh!

hamistao Mar 27, 2025

Uh oh!

dimaqq Mar 27, 2025

Uh oh!

dimaqq Mar 27, 2025

Uh oh!

hamistao Mar 27, 2025

Uh oh!

simondeziel commented Jun 27, 2025

Uh oh!

auvipy commented Jun 28, 2025

Uh oh!

simondeziel commented Jun 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants



		class _WebsocketClient(WebSocketBaseClient):
		class _WebsocketClient(ClientConnection):

		# PROTOCOL_TLS_CLIENT does not exist on older Python versions
		protocol = getattr(ssl, "PROTOCOL_TLS_CLIENT", ssl.PROTOCOL_TLSv1_2)

Switch from ws4py to websockets #626

Switch from ws4py to websockets #626

Uh oh!

Conversation

hamistao commented Jan 13, 2025

Uh oh!

hamistao commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 19, 2025

Codecov Report

Uh oh!

dimaqq Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

hamistao Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

dimaqq Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

dimaqq Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

hamistao Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

simondeziel commented Jun 27, 2025

Uh oh!

auvipy commented Jun 28, 2025

Uh oh!

simondeziel commented Jun 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Switch from `ws4py` to `websockets` #626

Switch from `ws4py` to `websockets` #626

hamistao commented Jan 14, 2025 •

edited

Loading