Skip to content

Upgrade awxkit websockets extra to websocket-client 1.x - #397

Merged
cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:fix/awxkit-websocket-client
Jun 13, 2026
Merged

Upgrade awxkit websockets extra to websocket-client 1.x#397
cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:fix/awxkit-websocket-client

Conversation

@blaipr

@blaipr blaipr commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

The awxkit websockets extra was pinned to websocket-client 0.57.0, a release from 2019 that has been end of life for years.

Just bumping the pin would not have worked. Starting with websocket-client 1.0, the library calls the WebSocketApp callbacks with the app object as the first argument. Our WSClient callbacks were still written in the old style, so under 1.x every callback raises a TypeError. The thread dies, the connection flag never gets set, and connect() fails with a misleading "Failed to establish channel connection w/ AWX" error.

This is not a theoretical problem. The AWX virtualenv already contains websocket-client 1.7.0, pulled in by the kubernetes package. So anyone using WSClient inside that environment today is running broken code. The same was true of awxkit's own tox environment, which installs websocket-client unpinned: the tests have been running against 1.x for a long time and nothing failed, because no test ever exercised the callbacks.

What this PR does:

  • updates the four callback signatures to the 1.x convention (on_close now also receives the close status code and message)
  • sets the extra to websocket-client>=1.0.0, so a constrained environment cannot resolve an old 0.x release against the new signatures
  • adds tests that call the callbacks exactly the way run_forever does, so a future change in the calling convention fails loudly instead of silently
  • removes a leftover: connect() was passing self.ws into a thread argument that was never used

How it was verified: the awxkit test suite passes (211 tests, including the new ones), and I ran a live session against a running AWX with websocket-client 1.7.0: connect, subscribe, receive the unsubscribe acknowledgement through _on_message, close cleanly.

One thing I found along the way and left alone: the server only accepts session cookie auth for websockets, so WSClient's token cookie option gets a 403 against current AWX no matter which client version is installed. That predates this change.

ISSUE TYPE

  • Bug, Docs Fix or other nominal change

COMPONENT NAME

  • CLI

ASCENDER VERSION

awx: 25.4.1.dev11+g51188f0

ADDITIONAL INFORMATION

py.test awxkit/test/  # 211 passed (cli/ modules skipped, pre-existing metadata issue in the dev env)

Independent of the other open PRs, verified with git merge-tree.

@blaipr
blaipr force-pushed the fix/awxkit-websocket-client branch from 7f74694 to 7da7da4 Compare June 12, 2026 16:07
@cigamit
cigamit requested a review from Copilot June 12, 2026 17:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates awxkit’s websocket client integration to be compatible with websocket-client 1.x by aligning WSClient callback signatures with the 1.x callback convention, and adds unit tests to prevent regressions.

Changes:

  • Update WSClient’s WebSocketApp callback method signatures (_on_open, _on_message, _on_error, _on_close) to match websocket-client 1.x invocation style.
  • Adjust the websocket thread runner wiring (connect() / _ws_run_forever()) to match the new callback integration approach.
  • Add dedicated unit tests that invoke callbacks exactly as run_forever() does under websocket-client 1.x.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
awxkit/awxkit/ws.py Updates websocket callback signatures and thread runner to match websocket-client 1.x behavior.
awxkit/test/test_ws.py Adds unit tests to lock in the websocket-client 1.x callback invocation convention and unsubscribe-ack behavior.
awxkit/setup.py Removes the old websocket-client==0.57.0 pin from the websockets extra.

Comment thread awxkit/setup.py Outdated
The websockets extra pinned websocket-client==0.57.0 (2019, long EOL).
websocket-client 1.x invokes WebSocketApp callbacks with the app as the
first argument; WSClient's callbacks still used the 0.x style, so a
plain pin bump would have broken every callback at runtime. The
signatures now follow the 1.x convention (on_close also receives the
close status code and message) and the extra requires
websocket-client>=1.0.0 - the floor matters because environments with
global constraints could otherwise still resolve a 0.x release against
the new signatures.

awxkit's tox environment already installed unpinned websocket-client
(latest 1.x), so it had been running 1.x against 0.x-convention code
unnoticed, because no test exercised the callbacks. The new tests
invoke the callbacks exactly as run_forever does (including a caplog
assertion on the error path), enforcing the convention regardless of
which 1.x release resolves.

Also drops the unused sockopt thread argument connect() passed to
_ws_run_forever.

Verified against websocket-client 1.7.0 with the new unit tests plus a
live round trip against a running AWX (connect, subscribe, unsubscribe
ack received through _on_message, clean close).
@blaipr
blaipr force-pushed the fix/awxkit-websocket-client branch from 7da7da4 to 45b5396 Compare June 12, 2026 17:36
@blaipr

blaipr commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Good point. I put the floor back, the extra now requires websocket-client>=1.0.0. Without it, an environment with a constraints file could still install an old 0.x release, and the new callback code would break at runtime. The tests cover the calling convention itself, the floor just makes sure pip cannot hand you a library that never used it.

@cigamit

cigamit commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Small issue with the eslint blocks for navigate. When you add a comment like this

    // eslint-disable-next-line react-hooks/exhaustive-deps -- navigate is not
    // referentially stable in react-router-dom-v5-compat; including it refires
    // this redirect effect after unrelated navigations
 }, [inventoryId]);

eslint is still throwing the error on the line, because the next line is actually a comment, not code.

    // navigate is not referentially stable in react-router-dom-v5-compat;
    // including it refires this redirect effect after unrelated navigations
    // eslint-disable-next-line react-hooks/exhaustive-deps

Whereas this is working properly and esline doesn't throw an error about navigate not being referenced.

@blaipr

blaipr commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Good catch — the wrapped explanation made the directive target the next comment line instead of the dependency array. Fixed in #392 (the PR those blocks belong to) with blaipr/ascender@876d8ef: the explanation now sits above a single-line bare eslint-disable-next-line, exactly as you suggested. All 8 affected files lint clean.

@cigamit
cigamit merged commit 879ce5e into ctrliq:main Jun 13, 2026
@cigamit cigamit self-assigned this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants