From 609b05b4ee009fff24e6b9c01b84690f7a11c93c Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 19 Oct 2023 16:17:34 +1300 Subject: [PATCH] docs: document more of the exceptions that may be raised (#1044) ### More extensively document the exceptions that Charmers should be aware of and potentially catching If we raise an exception when checking an argument's type, and the type hinting covers this, I have not included it in the `Raises` list. I feel that the docs (and a type checker) cover this already, and the `Raises` section should be more focused on exceptions that the Charmer should consider handling. #908 goes back-and-forth on documenting very common exceptions (like `APIError` and `CommunicationError` in nearly all pebble `Client` methods), and I have as well. ~I'm currently +0 on documenting them, and it seems like it's simpler to add them all in the PR and them remove them than decide to add them after the initial PR, so they are included at the moment. I could easily be convinced that they should be removed and a central doc note added (but I'm not sure where that would be best).~ These have been removed and are centralised in the `Container` and `pebble.Client` docstrings. Exceptions that are possible but should be rare and indicate a broken system (such as Juju or Pebble returning invalid data or being unresponsive) are not included. It feels like this tips *too* far in the "document everything" direction, and I don't feel like it would be worth Charmers trying to handle these exceptional (no pun intended) circumstances. Where it was unclear whether it was worth documenting a potential exception - such as not providing any commands in `exec`, which seems more likely to be a coding error than something the Charm should handle, but I could see happening with quite dynamic code - I have generally taken the more defensive approach of including the potential raise. I wrote a terrible, horrible, no good, very bad charm that causes a large number of these errors, and examined the pebble & Juju code to find information about what might cause issues. However, since I'm still only lightly familiar with Juju/ops, I could have missed some cases. ### Minor tweaks to exception chaining * If we are raising an exception in an exception handler but the original exception is just an implementation detail, `raise x from None` * If we are converting an exception to a more specific/localised one and the original exception is relevant, `raise x from e` * If we are filtering an exception and then re-raising, `raise` or `raise e` (preferably the former, but leaving the latter if already in place) ### Miscellanous One minor type hinting improvement: if an exception is always raised, use `typing.NoReturn` as the return type. Drive-bys: * Remove one `type: ignore` that is no longer required. * Add `ops.` to a couple of examples in the docstrings that were missed in #921 Fixes #908, #1043 --- HACKING.md | 2 ++ ops/charm.py | 34 ++++++++++++++++++---- ops/framework.py | 21 ++++++++----- ops/model.py | 59 ++++++++++++++++++++++++++++++++----- ops/pebble.py | 76 +++++++++++++++++++++++++++++++++--------------- ops/storage.py | 4 +-- ops/testing.py | 47 ++++++++++++++++++------------ 7 files changed, 178 insertions(+), 65 deletions(-) diff --git a/HACKING.md b/HACKING.md index 189a2aa1b..85a046859 100644 --- a/HACKING.md +++ b/HACKING.md @@ -105,6 +105,8 @@ Currently we don't publish separate versions of documentation for separate relea next to the relevant content (e.g. headings, etc.). +Noteworthy changes should also get a new entry in [CHANGES.md](CHANGES.md). + ## Dependencies diff --git a/ops/charm.py b/ops/charm.py index 3263ddce2..079744841 100755 --- a/ops/charm.py +++ b/ops/charm.py @@ -25,6 +25,7 @@ List, Literal, Mapping, + NoReturn, Optional, TextIO, Tuple, @@ -120,11 +121,14 @@ class ActionEvent(EventBase): params: Dict[str, Any] """The parameters passed to the action.""" - def defer(self) -> None: + def defer(self) -> NoReturn: """Action events are not deferrable like other events. This is because an action runs synchronously and the administrator is waiting for the result. + + Raises: + RuntimeError: always. """ raise RuntimeError('cannot defer action events') @@ -180,6 +184,13 @@ def set_results(self, results: Dict[str, Any]): Args: results: The result of the action as a Dict + + Raises: + ModelError: if a reserved key is used. + ValueError: if ``results`` has a mix of dotted/non-dotted keys that expand out to + result in duplicate keys, for example: :code:`{'a': {'b': 1}, 'a.b': 2}`. Also + raised if a dict is passed with a key that fails to meet the format requirements. + OSError: if extremely large (>100KB) results are provided. """ self.framework.model._backend.action_set(results) @@ -364,8 +375,11 @@ def add_metrics(self, metrics: Mapping[str, Union[int, float]], Args: metrics: Key-value mapping of metrics that have been gathered. labels: Key-value labels applied to the metrics. + + Raises: + ModelError: if invalid keys or values are provided. """ - self.framework.model._backend.add_metrics(metrics, labels) # type:ignore + self.framework.model._backend.add_metrics(metrics, labels) class RelationEvent(HookEvent): @@ -767,8 +781,12 @@ class SecretRotateEvent(SecretEvent): revision by calling :meth:`event.secret.set_content() `. """ - def defer(self) -> None: - """Secret rotation events are not deferrable (Juju handles re-invocation).""" + def defer(self) -> NoReturn: + """Secret rotation events are not deferrable (Juju handles re-invocation). + + Raises: + RuntimeError: always. + """ raise RuntimeError( 'Cannot defer secret rotation events. Juju will keep firing this ' 'event until you create a new revision.') @@ -847,8 +865,12 @@ def restore(self, snapshot: Dict[str, Any]): super().restore(snapshot) self._revision = cast(int, snapshot['revision']) - def defer(self) -> None: - """Secret expiration events are not deferrable (Juju handles re-invocation).""" + def defer(self) -> NoReturn: + """Secret expiration events are not deferrable (Juju handles re-invocation). + + Raises: + RuntimeError: always. + """ raise RuntimeError( 'Cannot defer secret expiration events. Juju will keep firing ' 'this event until you create a new revision.') diff --git a/ops/framework.py b/ops/framework.py index 54cf85778..e1be6b1a8 100755 --- a/ops/framework.py +++ b/ops/framework.py @@ -263,11 +263,11 @@ class EventSource: It is generally used as:: - class SomethingHappened(EventBase): + class SomethingHappened(ops.EventBase): pass class SomeObject(Object): - something_happened = EventSource(SomethingHappened) + something_happened = ops.EventSource(SomethingHappened) With that, instances of that type will offer the ``someobj.something_happened`` attribute which is a :class:`BoundEvent`, and may be used to emit and observe @@ -445,6 +445,10 @@ def define_event(cls, event_kind: str, event_type: 'Type[EventBase]'): event. Must be a valid Python identifier, not be a keyword or an existing attribute. event_type: A type of the event to define. + + Raises: + RuntimeError: if the same event is defined twice, or if ``event_kind`` + is an invalid name. """ prefix = 'unable to define an event with event_kind that ' if not event_kind.isidentifier(): @@ -710,7 +714,7 @@ def save_snapshot(self, value: Union["StoredStateData", "EventBase"]): marshal.dumps(data) except ValueError: msg = "unable to save the data for {}, it must contain only simple types: {!r}" - raise ValueError(msg.format(value.__class__.__name__, data)) + raise ValueError(msg.format(value.__class__.__name__, data)) from None self._storage.save_snapshot(value.handle.path, data) @@ -954,6 +958,9 @@ def breakpoint(self, name: Optional[str] = None): stop execution when a hook event is about to be handled. For those reasons, the "all" and "hook" breakpoint names are reserved. + + Raises: + ValueError: if the breakpoint name is invalid. """ # If given, validate the name comply with all the rules if name is not None: @@ -1099,15 +1106,15 @@ class StoredState: Example:: - class MyClass(Object): - _stored = StoredState() + class MyClass(ops.Object): + _stored = ops.StoredState() Instances of ``MyClass`` can transparently save state between invocations by setting attributes on ``_stored``. Initial state should be set with ``set_default`` on the bound object, that is:: - class MyClass(Object): - _stored = StoredState() + class MyClass(ops.Object): + _stored = ops.StoredState() def __init__(self, parent, key): super().__init__(parent, key) diff --git a/ops/model.py b/ops/model.py index 12dd2bece..29ec6f53d 100644 --- a/ops/model.py +++ b/ops/model.py @@ -354,7 +354,7 @@ def status(self) -> 'StatusBase': Example:: - self.model.app.status = BlockedStatus('I need a human to come help me') + self.model.app.status = ops.BlockedStatus('I need a human to come help me') """ if not self._is_our_app: return UnknownStatus() @@ -401,6 +401,8 @@ def planned_units(self) -> int: This method only returns data for this charm's application -- the Juju agent isn't able to see planned unit counts for other applications in the model. + Raises: + RuntimeError: on trying to get the planned units for a remote application. """ if not self._is_our_app: raise RuntimeError( @@ -431,6 +433,9 @@ def add_secret(self, content: Dict[str, str], *, rotate: Rotation policy/time. Every time this elapses, Juju will notify the charm by sending a SecretRotate event. None (the default) means to use the Juju default, which is never rotate. + + Raises: + ValueError: if the secret is empty, or the secret key is invalid. """ Secret._validate_content(content) id = self._backend.secret_add( @@ -506,9 +511,10 @@ def status(self) -> 'StatusBase': RuntimeError: if setting the status of a unit other than the current unit InvalidStatusError: if setting the status to something other than a :class:`StatusBase` + Example:: - self.model.unit.status = MaintenanceStatus('reconfiguring the frobnicators') + self.model.unit.status = ops.MaintenanceStatus('reconfiguring the frobnicators') """ if not self._is_our_unit: return UnknownStatus() @@ -567,7 +573,11 @@ def set_workload_version(self, version: str) -> None: @property def containers(self) -> Mapping[str, 'Container']: - """Return a mapping of containers indexed by name.""" + """Return a mapping of containers indexed by name. + + Raises: + RuntimeError: if called for another unit + """ if not self._is_our_unit: raise RuntimeError(f'cannot get container for a remote unit {self}') return self._containers @@ -581,7 +591,7 @@ def get_container(self, container_name: str) -> 'Container': try: return self.containers[container_name] except KeyError: - raise ModelError(f'container {container_name!r} not found') + raise ModelError(f'container {container_name!r} not found') from None def add_secret(self, content: Dict[str, str], *, label: Optional[str] = None, @@ -591,6 +601,9 @@ def add_secret(self, content: Dict[str, str], *, """Create a :class:`Secret` owned by this unit. See :meth:`Application.add_secret` for parameter details. + + Raises: + ValueError: if the secret is empty, or the secret key is invalid. """ Secret._validate_content(content) id = self._backend.secret_add( @@ -676,6 +689,11 @@ def set_ports(self, *ports: Union[int, 'Port']) -> None: Args: ports: The ports to open. Provide an int to open a TCP port, or a :class:`Port` to open a port for another protocol. + + Raises: + ModelError: if a :class:`Port` is provided where ``protocol`` is 'icmp' but + ``port`` is not ``None``, or where ``protocol`` is 'tcp' or 'udp' and ``port`` + is ``None``. """ # Normalise to get easier comparisons. existing = { @@ -1821,6 +1839,9 @@ def fetch(self, name: str) -> Path: If successfully fetched, this returns the path where the resource is stored on disk, otherwise it raises a :class:`NameError`. + + Raises: + NameError: if the resource's path cannot be fetched. """ if name not in self._paths: raise NameError(f'invalid resource name: {name}') @@ -1890,6 +1911,9 @@ def request(self, storage_name: str, count: int = 1): Uses storage-add tool to request additional storage. Juju will notify the unit via ``-storage-attached`` events when it becomes available. + + Raises: + ModelError: if the storage is not in the charm's metadata. """ if storage_name not in self._storage_map: raise ModelError(('cannot add storage {!r}:' @@ -1983,6 +2007,17 @@ class Container: This class should not be instantiated directly, instead use :meth:`Unit.get_container` or :attr:`Unit.containers`. + + For methods that make changes to the container, if the change fails or times out, then a + :class:`ops.pebble.ChangeError` or :class:`ops.pebble.TimeoutError` will be raised. + + Interactions with the container use Pebble, so all methods may raise exceptions when there are + problems communicating with Pebble. Problems connecting to or transferring data with Pebble + will raise a :class:`ops.pebble.ConnectionError` - generally you can guard against these by + first checking :meth:`can_connect`, but it is possible for problems to occur after + :meth:`can_connect` has succeeded. 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:`ops.pebble.APIError` is raised. """ name: str @@ -2110,7 +2145,8 @@ def get_services(self, *service_names: str) -> Mapping[str, 'pebble.ServiceInfo' def get_service(self, service_name: str) -> pebble.ServiceInfo: """Get status information for a single named service. - Raises :class:`ModelError` if service_name is not found. + Raises: + ModelError: if service_name is not found. """ services = self.get_services(service_name) if not services: @@ -2137,7 +2173,8 @@ def get_checks( def get_check(self, check_name: str) -> pebble.CheckInfo: """Get check information for a single named check. - Raises :class:`ModelError` if ``check_name`` is not found. + Raises: + ModelError: if ``check_name`` is not found. """ checks = self.get_checks(check_name) if not checks: @@ -2528,7 +2565,6 @@ def remove_path(self, path: Union[str, PurePath], *, recursive: bool = False): Raises: pebble.PathError: If a relative path is provided, or if `recursive` is False and the file or directory cannot be removed (it does not exist or is not empty). - """ self._pebble.remove_path(str(path), recursive=recursive) @@ -2598,6 +2634,13 @@ def exec( See :meth:`ops.pebble.Client.exec` for documentation of the parameters and return value, as well as examples. + + Note that older versions of Juju do not support the ``service_content`` parameter, so if + the Charm is to be used on those versions, then + :meth:`JujuVersion.supports_exec_service_context` should be used as a guard. + + Raises: + ExecError: if the command exits with a non-zero exit code. """ if service_context is not None: version = JujuVersion.from_environ() @@ -2873,7 +2916,7 @@ def _run(self, *args: str, return_output: bool = False, try: result = subprocess.run(args, **kwargs) # type: ignore except subprocess.CalledProcessError as e: - raise ModelError(e.stderr) + raise ModelError(e.stderr) from e if return_output: if result.stdout is None: # type: ignore return '' diff --git a/ops/pebble.py b/ops/pebble.py index 80ce8f6e7..e1e8e3648 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -1344,6 +1344,7 @@ def wait_output(self) -> Tuple[AnyStr, Optional[AnyStr]]: Raises: ChangeError: if there was an error starting or running the process. ExecError: if the process exits with a non-zero exit code. + TypeError: if :meth:`Client.exec` was called with the ``stdout`` argument. """ if self.stdout is None: raise TypeError( @@ -1528,6 +1529,15 @@ class Client: Defaults to using a Unix socket at socket_path (which must be specified unless a custom opener is provided). + + For methods that wait for changes, such as :meth:`start_services` and :meth:`replan_services`, + if the change fails or times out, then a :class:`ChangeError` or :class:`TimeoutError` will be + raised. + + All methods may raise exceptions when there are problems communicating with Pebble. Problems + 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. """ _chunk_size = 8192 @@ -1621,13 +1631,13 @@ def _request_raw( # Will only happen on read error or if Pebble sends invalid JSON. body: Dict[str, Any] = {} message = f'{type(e2).__name__} - {e2}' - raise APIError(body, code, status, message) + raise APIError(body, code, status, message) from None except urllib.error.URLError as e: if e.args and isinstance(e.args[0], FileNotFoundError): raise ConnectionError( f"Could not connect to Pebble: socket not found at {self.socket_path!r} " "(container restarted?)") from None - raise ConnectionError(e.reason) + raise ConnectionError(e.reason) from e return response @@ -1673,16 +1683,16 @@ def autostart_services(self, timeout: float = 30.0, delay: float = 0.1) -> Chang """Start the startup-enabled services and wait (poll) for them to be started. Args: - timeout: Seconds before autostart change is considered timed out (float). + timeout: Seconds before autostart change is considered timed out (float). If + timeout is 0, submit the action but don't wait; just return the change ID + immediately. delay: Seconds before executing the autostart change (float). Returns: ChangeID of the autostart change. Raises: - ChangeError: if one or more of the services didn't start. If - timeout is 0, submit the action but don't wait; just return the change - ID immediately. + ChangeError: if one or more of the services didn't start, and ``timeout`` is non-zero. """ return self._services_action('autostart', [], timeout, delay) @@ -1690,16 +1700,17 @@ def replan_services(self, timeout: float = 30.0, delay: float = 0.1) -> ChangeID """Replan by (re)starting changed and startup-enabled services and wait for them to start. Args: - timeout: Seconds before replan change is considered timed out (float). + timeout: Seconds before replan change is considered timed out (float). If + timeout is 0, submit the action but don't wait; just return the change + ID immediately. delay: Seconds before executing the replan change (float). Returns: ChangeID of the replan change. Raises: - ChangeError: if one or more of the services didn't stop/start. If - timeout is 0, submit the action but don't wait; just return the change - ID immediately. + ChangeError: if one or more of the services didn't stop/start, and ``timeout`` is + non-zero. """ return self._services_action('replan', [], timeout, delay) @@ -1710,16 +1721,17 @@ def start_services( Args: services: Non-empty list of services to start. - timeout: Seconds before start change is considered timed out (float). + timeout: Seconds before start change is considered timed out (float). If + timeout is 0, submit the action but don't wait; just return the change + ID immediately. delay: Seconds before executing the start change (float). Returns: ChangeID of the start change. Raises: - ChangeError: if one or more of the services didn't stop/start. If - timeout is 0, submit the action but don't wait; just return the change - ID immediately. + ChangeError: if one or more of the services didn't stop/start, and ``timeout`` is + non-zero. """ return self._services_action('start', services, timeout, delay) @@ -1730,16 +1742,17 @@ def stop_services( Args: services: Non-empty list of services to stop. - timeout: Seconds before stop change is considered timed out (float). + timeout: Seconds before stop change is considered timed out (float). If + timeout is 0, submit the action but don't wait; just return the change + ID immediately. delay: Seconds before executing the stop change (float). Returns: ChangeID of the stop change. Raises: - ChangeError: if one or more of the services didn't stop/start. If - timeout is 0, submit the action but don't wait; just return the change - ID immediately. + ChangeError: if one or more of the services didn't stop/start and ``timeout`` is + non-zero. """ return self._services_action('stop', services, timeout, delay) @@ -1750,16 +1763,17 @@ def restart_services( Args: services: Non-empty list of services to restart. - timeout: Seconds before restart change is considered timed out (float). + timeout: Seconds before restart change is considered timed out (float). If + timeout is 0, submit the action but don't wait; just return the change + ID immediately. delay: Seconds before executing the restart change (float). Returns: ChangeID of the restart change. Raises: - ChangeError: if one or more of the services didn't stop/start. If - timeout is 0, submit the action but don't wait; just return the change - ID immediately. + ChangeError: if one or more of the services didn't stop/start and ``timeout`` is + non-zero. """ return self._services_action('restart', services, timeout, delay) @@ -2021,6 +2035,10 @@ def push( group_id: Group ID (GID) for file. group: Group name for file. Group's GID must match group_id if both are specified. + + Raises: + PathError: If there was an error writing the file to the path; for example, if the + destination path doesn't exist and ``make_dirs`` is not used. """ info = self._make_auth_dict(permissions, user_id, user, group_id, group) info['path'] = path @@ -2118,6 +2136,10 @@ def list_files(self, path: str, *, pattern: Optional[str] = None, for example ``*.txt``. itself: If path refers to a directory, return information about the directory itself, rather than its contents. + + Raises: + PathError: if there was an error listing the directory; for example, if the directory + does not exist. """ query = { 'action': 'list', @@ -2151,6 +2173,10 @@ def make_dir( group_id: Group ID (GID) for directory. group: Group name for directory. Group's GID must match group_id if both are specified. + + Raises: + PathError: if there was an error making the directory; for example, if the parent path + does not exist, and ``make_parents`` is not used. """ info = self._make_auth_dict(permissions, user_id, user, group_id, group) info['path'] = path @@ -2176,7 +2202,6 @@ def remove_path(self, path: str, *, recursive: bool = False): Raises: pebble.PathError: If a relative path is provided, or if `recursive` is False and the file or directory cannot be removed (it does not exist or is not empty). - """ info: Dict[str, Any] = {'path': path} if recursive: @@ -2368,6 +2393,11 @@ def exec( :meth:`ExecProcess.wait` if stdout/stderr were provided as arguments to :meth:`exec`, or :meth:`ExecProcess.wait_output` if not. + + Raises: + APIError: if an error occurred communicating with pebble, or if the command is not + found. + ExecError: if the command exits with a non-zero exit code. """ if not isinstance(command, list) or not all(isinstance(s, str) for s in command): raise TypeError(f'command must be a list of str, not {type(command).__name__}') diff --git a/ops/storage.py b/ops/storage.py index d8ddacbd5..5dda21d0e 100755 --- a/ops/storage.py +++ b/ops/storage.py @@ -104,7 +104,7 @@ def save_snapshot(self, handle_path: str, snapshot_data: Any) -> None: Args: handle_path: The string identifying the snapshot. snapshot_data: The data to be persisted. (as returned by Object.snapshot()). This - might be a dict/tuple/int, but must only contain 'simple' python types. + might be a dict/tuple/int, but must only contain 'simple' Python types. """ # Use pickle for serialization, so the value remains portable. raw_data = pickle.dumps(snapshot_data) @@ -235,7 +235,7 @@ def load_snapshot(self, handle_path: str): try: content = self._backend.get(handle_path) except KeyError: - raise NoSnapshotError(handle_path) + raise NoSnapshotError(handle_path) from None return content def drop_snapshot(self, handle_path: str): diff --git a/ops/testing.py b/ops/testing.py index a8f830f34..c9af7832d 100755 --- a/ops/testing.py +++ b/ops/testing.py @@ -305,6 +305,8 @@ def begin(self) -> None: Before calling :meth:`begin`, there is no Charm instance, so changes to the Model won't emit events. Call :meth:`.begin` for :attr:`.charm` to be valid. + + Should only be called once. """ if self._charm is not None: raise RuntimeError('cannot call the begin method on the harness more than once') @@ -542,9 +544,9 @@ def _get_config(self, charm_config_yaml: Optional['YAMLStringOrFile']): def add_oci_resource(self, resource_name: str, contents: Optional[Mapping[str, str]] = None) -> None: - """Add oci resources to the backend. + """Add OCI resources to the backend. - This will register an oci resource and create a temporary file for processing metadata + This will register an OCI resource and create a temporary file for processing metadata about the resource. A default set of values will be used for all the file contents unless a specific contents dict is provided. @@ -577,7 +579,7 @@ def add_resource(self, resource_name: str, content: AnyStr) -> None: returned by resource-get. If contents is a string, it will be encoded in utf-8 """ if resource_name not in self._meta.resources.keys(): - raise RuntimeError(f'Resource {resource_name} is not a defined resources') + raise RuntimeError(f'Resource {resource_name} is not a defined resource') record = self._meta.resources[resource_name] if record.type != "file": raise RuntimeError( @@ -679,6 +681,9 @@ def detach_storage(self, storage_id: str) -> None: It will trigger a storage-detaching hook if the storage unit in question exists and is presently marked as attached. + Note that the Charm is not instantiated until :meth:`begin` is called. + Until then, attempting to use this method will raise an exception. + Args: storage_id: The full storage ID of the storage unit being detached, including the storage key, e.g. my-storage/0. @@ -735,6 +740,9 @@ def remove_storage(self, storage_id: str) -> None: Args: storage_id: The full storage ID of the storage unit being removed, including the storage key, e.g. my-storage/0. + + Raises: + RuntimeError: if the storage is not in the metadata. """ storage_name, storage_index = storage_id.split('/', 1) storage_index = int(storage_index) @@ -893,9 +901,6 @@ def add_relation_unit(self, relation_id: int, remote_unit_name: str) -> None: Args: relation_id: The integer relation identifier (as returned by :meth:`add_relation`). remote_unit_name: A string representing the remote unit that is being added. - - Return: - None """ self._backend._relation_list_map[relation_id].append(remote_unit_name) # we can write remote unit data iff we are not in a hook env @@ -1035,9 +1040,11 @@ def get_container_pebble_plan( Return: The Pebble plan for this container. Use :meth:`Plan.to_yaml ` to get a string - form for the content. Will raise ``KeyError`` if no Pebble client - exists for that container name (should only happen if container is - not present in ``metadata.yaml``). + form for the content. + + Raises: + KeyError: if no Pebble client exists for that container name (should only happen if + container is not present in ``metadata.yaml``). """ client = self._backend._pebble_clients.get(container_name) if client is None: @@ -1241,6 +1248,9 @@ def update_config( unset: An iterable of keys to remove from config. This sets the value to the default if defined, otherwise removes the key altogether. + + Raises: + ValueError: if the key is not present in the config. """ self._update_config(key_values, unset) if self._charm is None or not self._hooks_enabled: @@ -1290,7 +1300,6 @@ def reset_planned_units(self) -> None: This allows the harness to fall through to the built in methods that will try to guess at a value for planned units, based on the number of peer relations that have been setup in the testing harness. - """ self._backend._planned_units = None @@ -1995,17 +2004,17 @@ def _get_resource_dir(self) -> pathlib.Path: def relation_ids(self, relation_name: str) -> List[int]: try: return self._relation_ids_map[relation_name] - except KeyError as e: + except KeyError: if relation_name not in self._meta.relations: - raise model.ModelError(f'{relation_name} is not a known relation') from e + raise model.ModelError(f'{relation_name} is not a known relation') from None no_ids: List[int] = [] return no_ids def relation_list(self, relation_id: int): try: return self._relation_list_map[relation_id] - except KeyError as e: - raise model.RelationNotFoundError from e + except KeyError: + raise model.RelationNotFoundError from None def relation_remote_app_name(self, relation_id: int) -> Optional[str]: if relation_id not in self._relation_app_and_units: @@ -2130,7 +2139,7 @@ def storage_get(self, storage_name_id: str, attribute: str) -> Any: return self._storage_list[name][index][attribute] except KeyError: raise model.ModelError( - f'ERROR invalid value "{name}/{index}" for option -s: storage not found') + f'ERROR invalid value "{name}/{index}" for option -s: storage not found') from None def storage_add(self, name: str, count: int = 1) -> List[int]: if '/' in name: @@ -2931,10 +2940,10 @@ def remove_path(self, path: str, *, recursive: bool = False): else: try: file_path.rmdir() - except OSError: + except OSError as e: raise pebble.PathError( 'generic-file-error', - 'cannot remove non-empty directory without recursive=True') + 'cannot remove non-empty directory without recursive=True') from e else: file_path.unlink() @@ -3041,7 +3050,7 @@ def exec( raise RuntimeError( "a TimeoutError occurred in the execution handler, " "but no timeout value was provided in the execution arguments." - ) + ) from None if result is None: exit_code = 0 proc_stdout = self._transform_exec_handler_output(b'', encoding) @@ -3105,7 +3114,7 @@ def send_signal(self, sig: Union[int, str], service_names: Iterable[str]): body=body, code=500, status='Internal Server Error', - message=message) + message=message) from None def get_checks(self, level=None, names=None): # type:ignore raise NotImplementedError(self.get_checks) # type:ignore