Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Bump websockets version #556

Closed
slothyrulez opened this issue Mar 11, 2024 · 18 comments · Fixed by #557
Closed

Question: Bump websockets version #556

slothyrulez opened this issue Mar 11, 2024 · 18 comments · Fixed by #557
Assignees

Comments

@slothyrulez
Copy link

slothyrulez commented Mar 11, 2024

Hi!
I was wondering if its feasible to bump the websockets lib dependency to >=11.0 instead of ^10.3

Not sure if the underlying changes affect to your library.

https://websockets.readthedocs.io/en/11.0.3/project/changelog.html

imagen

┆Issue is synchronized with this Jira Task by Unito

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 11, 2024

@slothyrulez may I ask what issues are you facing with ^10.3

@slothyrulez
Copy link
Author

slothyrulez commented Mar 11, 2024

@slothyrulez may I ask what issues are you facing with ^10.3

We have 3 dependencies that also install websockets in this case >=11.0 one of them being playwright

If I do not specify the ably version, ably=2.0.4, pip installs ably version 1.2.2

imagen

Thanks for asking @sacOO7

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 11, 2024

@slothyrulez I just checked the changelog and it seems to introduce breaking changes to public API of websocket library. Are you using this lib. in production ready app>

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 11, 2024

@slothyrulez One of the quick workaround I can suggest is to install ably first then install other 3 dependencies that use websockets

@slothyrulez
Copy link
Author

@slothyrulez I just checked the changelog and it seems to introduce breaking changes to public API of websocket library. Are you using this lib. in production ready app>

This is our intend.

@slothyrulez
Copy link
Author

@slothyrulez One of the quick workaround I can suggest is to install ably first then install other 3 dependencies that use websockets

Sadly this does not seem to work. It immediately collides when try to install the next lib using websockets
imagen

@slothyrulez
Copy link
Author

@slothyrulez I just checked the changelog and it seems to introduce breaking changes to public API of websocket library. Are you using this lib. in production ready app>

That's true, but also seems that websockets has keep the old public API for retro-compatibility, those changes, also (naively on my side) seem not to affect your lib.

@slothyrulez
Copy link
Author

slothyrulez commented Mar 11, 2024

I don't want to lose your time anymore, I've cloned the repo, and updated the lib. and the tests are passing. I know this doesn't mean upgrading is safe, but is a good sign. Not sure if i can help you with anything. My hope here is that may be you consider adding a bump on the next release. thanks.

imagen
imagen

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 12, 2024

@slothyrulez okay it seems websockets 11.0 was made available on Apr 2 2023, so makes sense to upgrade the library.

@sacOO7 sacOO7 self-assigned this Mar 12, 2024
@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 12, 2024

@slothyrulez I have created PR #557 for this fix. Can you check if this will work for you

@slothyrulez
Copy link
Author

@slothyrulez okay it seems websockets 11.0 was made available on Apr 2 2023, so makes sense to upgrade the library.

That's huge news! thanks @sacOO7

@slothyrulez
Copy link
Author

slothyrulez commented Mar 12, 2024

@slothyrulez I have created PR #557 for this fix. Can you check if this will work for you

Installed perfectly now:

imagen

it also bumped websockets to 12.0 as expected.

imagen

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 12, 2024

@slothyrulez good to hear that! Will post here once PR is merged and new release is made : )

@slothyrulez
Copy link
Author

slothyrulez commented Mar 12, 2024

@slothyrulez good to hear that! Will post here once PR is merged and new release is made : )

Outstanding @sacOO7 , sadly seems that the sync interface is not built installing this way, no worries I'll search a workaround and/or wait until you release the new version.

imagen

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 12, 2024

@slothyrulez That should be available after release. Meanwhile, you can generate sync version locally by going into the installed package and run

poetry run unasync

or try at root

poetry ably run unasync // ably is a module name here, not sure this will work though 

@slothyrulez
Copy link
Author

slothyrulez commented Mar 12, 2024

@slothyrulez That should be available after release. Meanwhile, you can generate sync version locally by going into the installed package and run

poetry run unasync

or try at root

poetry ably run unasync // ably is a module name here, not sure this will work though 

Installed your branch as editable (-e) installed the dev dependencies and executed the command in CONTRIBUTING.md (poetry run unasync) and the code is there now, thanks again.

@slothyrulez
Copy link
Author

@sacOO7 Thanks for your kind support!

@sacOO7
Copy link
Collaborator

sacOO7 commented Mar 12, 2024

No issues, we will release this by tomorrow 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants