Idea #21146
closed
Replace ws4py dependency from PySDK
Added by Brett Smith over 1 year ago.
Updated 12 months ago.
Release relationship:
Auto
Description
The GitHub project says "[Project on Hiatus]"
The last commit just removed a similar warning from the README.
This is becoming a liability because the project build structure relies on a stack that's more and more deprecated. It's becoming harder to install on newer distributions and that problem is only going to get worse over time.
Find and switch to a replacement while maintaining the API of arvados.events
and other affected modules.
- Target version changed from Future to Development 2024-01-03 sprint
The good news is we built our own class that Has-A client underneath, rather than subclassing. Thanks to that, here is the entire API we need to support.
__init__
should start a daemon thread to run the connection.
- Each
f
argument is a filter.
class EventClient(object):
def __init__(self, url, filters, on_event_cb, last_log_id): ...
def subscribe(self, f, last_log_id=None): ...
def unsubscribe(self, f): ...
def close(self, code=1000, reason='', timeout=0): ...
def on_event(self, m): ...
def on_closed(self): ...
def run_forever(self): ...
We can use the websockets package for this. It has a thread-based client that's pretty compatible with our needs. The main difference is that the API is not callback-oriented, so we'll need to write that glue code ourselves, but that's not hard.
The license is 3-clause BSD so it's compatible with our SDK. Support is available through Tidelift so it's a mature project. Last release was a month ago so it's actively maintained. And of course it's the top result when you search "websockets" on PyPI.
- Assigned To set to Brett Smith
- Status changed from New to In Progress
I have a branch that passes existing tests. I need to add a couple tests (due to different behavior in the new library) and docs.
21146-pysdk-new-websockets @ 187f790d8b71c5c7b54a599bec6e58950d7116dc - developer-run-tests: #3938 
remainder rerun developer-run-tests-remainder: #4142
- This has a different failure, so the intersection of both runs pass all tests 🙄
- All agreed upon points are implemented / addressed.
- Anything not implemented (discovered or discussed during work) has a follow-up story.
- Code is tested and passing, both automated and manual, what manual testing was done is described
- Documentation has been updated.
- Behaves appropriately at the intended scale (describe intended scale).
- I have not benchmarked to compare performance of ws4py vs. websockets or whatever, but it seems unlikely to be a significant difference.
- Considered backwards and forwards compatibility issues between client and server.
- The new EventClient maintains its entire public API, including methods, argument names, attributes, and semantics. Some of the code organization came about specifically to maintain semantics. For example, I thought about moving the
last_log_id
updating from on_event
to run
, but then weird things might've happened if someone subclassed EventClient with their own on_event
.
- Follows our coding standards and GUI style guidelines.
Brett Smith wrote in #note-6:
21146-pysdk-new-websockets @ 187f790d8b71c5c7b54a599bec6e58950d7116dc - developer-run-tests: #3938 
remainder rerun developer-run-tests-remainder: #4142
- This has a different failure, so the intersection of both runs pass all tests 🙄
- All agreed upon points are implemented / addressed.
- Anything not implemented (discovered or discussed during work) has a follow-up story.
- Code is tested and passing, both automated and manual, what manual testing was done is described
- Documentation has been updated.
- Behaves appropriately at the intended scale (describe intended scale).
- I have not benchmarked to compare performance of ws4py vs. websockets or whatever, but it seems unlikely to be a significant difference.
- Considered backwards and forwards compatibility issues between client and server.
- The new EventClient maintains its entire public API, including methods, argument names, attributes, and semantics. Some of the code organization came about specifically to maintain semantics. For example, I thought about moving the
last_log_id
updating from on_event
to run
, but then weird things might've happened if someone subclassed EventClient with their own on_event
.
- Follows our coding standards and GUI style guidelines.
This LGTM.
I did a little bit of manual testing with arv-ws
and it seems to be working fine.
- Status changed from In Progress to Resolved
Re putting this in 2.7.2: This branch wasn't written with Python 3.7 compatibility in mind. It hopefully wouldn't be a big deal to add, but it would need to be tested and maybe tweaked.
Per our previous discussion, this was yanked from 2.7.2 and will be in 3.0 instead.
Also available in: Atom
PDF