Skip to content

Commit

Permalink
Merge branch 'main' into changelog-changes
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyandrewmeyer authored Mar 26, 2024
2 parents e5d89c7 + e8305f6 commit ecf454d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* More robust validation of observer signatures (#1147)
* Change `Model.relation.app` type from `Application|None` to `Application` (#1151)
* Fix attaching storage in Harness before `begin` (#1150)
* Fixed an issue where `pebble.Client.exec` might leak a `socket.timeout` (`builtins.TimeoutError`) exception (#1155)

## Refactoring

Expand Down
12 changes: 10 additions & 2 deletions ops/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"""

import binascii
import builtins
import copy
import dataclasses
import datetime
Expand Down Expand Up @@ -1722,6 +1723,11 @@ class Client:
connecting to or transferring data with Pebble will raise a :class:`ConnectionError`. When an
error occurs executing the request, such as trying to add an invalid layer or execute a command
that does not exist, an :class:`APIError` is raised.
The ``timeout`` parameter specifies a timeout in seconds for blocking operations like the
connection attempt to Pebble; used by ``urllib.request.OpenerDirector.open``. It's not for
methods like :meth:`start_services` and :meth:`replan_services` mentioned above, and it's not
for the command execution timeout defined in method :meth:`Client.exec`.
"""

_chunk_size = 8192
Expand Down Expand Up @@ -2029,8 +2035,10 @@ def _wait_change_using_wait(self, change_id: ChangeID, timeout: Optional[float])

try:
return self._wait_change(change_id, this_timeout)
except TimeoutError:
# Catch timeout from wait endpoint and loop to check deadline
except (socket.timeout, builtins.TimeoutError):
# NOTE: in Python 3.10 socket.timeout is an alias of TimeoutError,
# but we still need to support Python 3.8, so catch both.
# Catch timeout from wait endpoint and loop to check deadline.
pass

raise TimeoutError(f'timed out waiting for change {change_id} ({timeout} seconds)')
Expand Down
13 changes: 13 additions & 0 deletions test/test_pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io
import json
import signal
import socket
import tempfile
import test.fake_pebble as fake_pebble
import typing
Expand Down Expand Up @@ -2028,6 +2029,18 @@ def test_wait_change_error(self):
('GET', '/v1/changes/70/wait', {'timeout': '4.000s'}, None),
])

def test_wait_change_socket_timeout(self):
def timeout_response(n: float):
self.time.sleep(n)
raise socket.timeout("socket.timeout: timed out")

self.client.responses.append(lambda: timeout_response(3))

with self.assertRaises(pebble.TimeoutError) as cm:
self.client.wait_change(pebble.ChangeID('70'), timeout=3)
self.assertIsInstance(cm.exception, pebble.Error)
self.assertIsInstance(cm.exception, TimeoutError)

def test_add_layer(self):
okay_response = {
"result": True,
Expand Down

0 comments on commit ecf454d

Please sign in to comment.