feat: move websocket-client to extra dependency#3123
Conversation
0af8e71 to
ba5f6b1
Compare
milas
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Out of curiosity, where/how are you using this library that you want to avoid the websocket-client dependency?
Technically, these changes look good. Thanks for the nice error message when it's not there. I'm not going to merge this right now, though: at a minimum, I want to do a major version bump along with this.
I'd also like to hear if others are interested in this (feel free to 👍🏻 this PR or comment with details), as it will potentially require a LOT of downstream consumers of this library to update their requirements.txt (or modern eqvuialent) since I think many do rely on websocket support when using the Engine API.
Also bump minimum version to that prescribed by docker#3022 Signed-off-by: Aarni Koskela <akx@iki.fi>
8319d56 to
c9799f8
Compare
|
@milas Hi, thanks for getting back on this! We have a situation where we'd like to avoid downloading extra dependencies in general – and from an attack surface/SBOM point of view, the less code there is, the better.
|
milas
left a comment
There was a problem hiding this comment.
Merging this - I'm going to do a major version bump to v7 for Python 3.12 compatibility, as I had to remove some old functionality in the process that constitutes a breaking change, so it's a good time to accept this as well.
Thanks for the PR!
|
@milas Great, thanks! Should v7 also drop Python 3.7 support? It's EOL date was 4 months ago. |
Since
websocket-clientis only required when using websockets for attaching to containers (#62), I don't think it needs to be a required dependency.This also bumps the minimum required version of
websocket-clientto that pinned by #3022.