Skip to content

Drop use of Monitor._ws.open and Connection._ws.closed#1208

Closed
freyes wants to merge 1 commit intojuju:mainfrom
freyes:websockets-fix
Closed

Drop use of Monitor._ws.open and Connection._ws.closed#1208
freyes wants to merge 1 commit intojuju:mainfrom
freyes:websockets-fix

Conversation

@freyes
Copy link
Copy Markdown
Contributor

@freyes freyes commented Nov 22, 2024

The websockets library has dropped the properties "open" and "closed" since websockets-13.0[0], this patch makes the changes needed to check for the connection's state as suggested by the documentation.

[0] python-websockets/websockets@7c8e0b9

Fixes #1207

The websockets library has dropped the properties "open" and "closed"
since websockets-13.0[0], this patch makes the changes needed to check
for the connection's state as suggested by the documentation.

[0] python-websockets/websockets@7c8e0b9
@dimaqq
Copy link
Copy Markdown
Contributor

dimaqq commented Nov 23, 2024

I imagine that would allow us to upgrade to websockets 14 or remove the dependency upper bound, wouldn’t it?

@dimaqq
Copy link
Copy Markdown
Contributor

dimaqq commented Nov 23, 2024

Pls reword commit messages to follow conventional commits

@dimaqq
Copy link
Copy Markdown
Contributor

dimaqq commented Nov 25, 2024

https://github.com/python-websockets/websockets/blob/59d4dcf779fe7d2b0302083b072d8b03adce2f61/src/websockets/protocol.py#L57-L60

The websockets library defines 4 values for State.

This PR references only two.

I wonder what about the other two.

self._debug_log_task.cancel()

if self._ws and not self._ws.closed:
if self._ws and self._ws.state is not State.CLOSED:
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.

Looking through websockets code, the library already handles the corner cases and we should close the connection unconditionally.

@dimaqq dimaqq mentioned this pull request Nov 25, 2024
freyes added a commit to freyes/zaza that referenced this pull request Nov 25, 2024
Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208
@dimaqq
Copy link
Copy Markdown
Contributor

dimaqq commented Nov 26, 2024

Continued in #1216

@dimaqq dimaqq closed this Nov 26, 2024
javacruft pushed a commit to openstack-charmers/zaza that referenced this pull request Nov 26, 2024
* Pin websockets library

Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208

* Migrate to actions/upload-artifact@v4
freyes added a commit to freyes/zaza that referenced this pull request Nov 26, 2024
* Pin websockets library

Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208

* Migrate to actions/upload-artifact@v4

(cherry picked from commit d758d6d)
freyes added a commit to freyes/zaza that referenced this pull request Nov 26, 2024
* Pin websockets library

Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208

* Migrate to actions/upload-artifact@v4

(cherry picked from commit d758d6d)
freyes added a commit to freyes/zaza that referenced this pull request Nov 26, 2024
* Pin websockets library

Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208

* Migrate to actions/upload-artifact@v4

(cherry picked from commit d758d6d)
(cherry picked from commit e8d72f4)
freyes added a commit to freyes/zaza that referenced this pull request Nov 26, 2024
* Pin websockets library

Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208

* Migrate to actions/upload-artifact@v4

(cherry picked from commit d758d6d)
(cherry picked from commit e8d72f4)
(cherry picked from commit 9b5291c)
freyes added a commit to freyes/zaza that referenced this pull request Nov 26, 2024
* Pin websockets library

Set upper bound for websockets until libjuju is compatible with newer
versions.

See juju/python-libjuju#1208

* Migrate to actions/upload-artifact@v4

(cherry picked from commit d758d6d)
(cherry picked from commit e8d72f4)
(cherry picked from commit 9b5291c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AttributeError: 'ClientConnection' object has no attribute 'open'

2 participants