Skip to content

Commit

Permalink
Add "-> None" to methods without other types (#961)
Browse files Browse the repository at this point in the history
This is so that if MyPy is used in a charm, it type checks them. MyPy
by default doesn't type check methods that have no type annotations on
them.
  • Loading branch information
benhoyt authored Jul 4, 2023
1 parent a747847 commit 384ef3a
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
6 changes: 3 additions & 3 deletions ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class ActionEvent(EventBase):
params: Dict[str, Any]
"""The parameters passed to the action."""

def defer(self):
def defer(self) -> None:
"""Action events are not deferrable like other events.
This is because an action runs synchronously and the administrator
Expand Down Expand Up @@ -737,7 +737,7 @@ class SecretRotateEvent(SecretEvent):
revision by calling :meth:`event.secret.set_content() <ops.Secret.set_content>`.
"""

def defer(self):
def defer(self) -> None:
"""Secret rotation events are not deferrable (Juju handles re-invocation)."""
raise RuntimeError(
'Cannot defer secret rotation events. Juju will keep firing this '
Expand Down Expand Up @@ -817,7 +817,7 @@ def restore(self, snapshot: Dict[str, Any]):
super().restore(snapshot)
self._revision = cast(int, snapshot['revision'])

def defer(self):
def defer(self) -> None:
"""Secret expiration events are not deferrable (Juju handles re-invocation)."""
raise RuntimeError(
'Cannot defer secret expiration events. Juju will keep firing '
Expand Down
25 changes: 13 additions & 12 deletions ops/framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def key(self) -> str:
return self._key

@property
def path(self):
def path(self) -> str:
"""Return the handle's path."""
return self._path

Expand Down Expand Up @@ -194,7 +194,7 @@ def __init__(self, handle: Handle):
def __repr__(self):
return f"<{self.__class__.__name__} via {self.handle}>"

def defer(self):
def defer(self) -> None:
"""Defer the event to the future.
Deferring an event from a handler puts that handler into a queue, to be
Expand Down Expand Up @@ -625,19 +625,20 @@ def __init__(self, storage: Union[SQLiteStorage, JujuStorage],
else:
self._juju_debug_at: Set[str] = set()

def set_breakpointhook(self):
"""Hook into sys.breakpointhook so the builtin breakpoint() works as expected.
def set_breakpointhook(self) -> Optional[Any]:
"""Hook into ``sys.breakpointhook`` so the builtin ``breakpoint()`` works as expected.
This method is called by ``main``, and is not intended to be
called by users of the framework itself outside of perhaps
some testing scenarios.
It returns the old value of sys.excepthook.
The breakpoint function is a Python >= 3.7 feature.
The ``breakpoint()`` function is a Python >= 3.7 feature.
This method was added in ops 1.0; before that, it was done as
part of the Framework's __init__.
part of the Framework's ``__init__``.
Returns:
The old value of ``sys.breakpointhook``.
"""
old_breakpointhook = getattr(sys, 'breakpointhook', None)
if old_breakpointhook is not None:
Expand All @@ -646,7 +647,7 @@ def set_breakpointhook(self):
sys.breakpointhook = self.breakpoint
return old_breakpointhook

def close(self):
def close(self) -> None:
"""Close the underlying backends."""
self._storage.close()

Expand All @@ -664,7 +665,7 @@ def _forget(self, obj: 'Serializable'):
"""Stop tracking the given object. See also _track."""
self._objects.pop(obj.handle.path, None)

def commit(self):
def commit(self) -> None:
"""Save changes to the underlying backends."""
# Give a chance for objects to persist data they want to before a commit is made.
self.on.pre_commit.emit()
Expand Down Expand Up @@ -831,7 +832,7 @@ def _emit(self, event: EventBase):
if saved:
self._reemit(event_path)

def reemit(self):
def reemit(self) -> None:
"""Reemit previously deferred events to the observers that deferred them.
Only the specific observers that have previously deferred the event will be
Expand Down Expand Up @@ -974,7 +975,7 @@ def breakpoint(self, name: Optional[str] = None):
"Breakpoint %r skipped (not found in the requested breakpoints: %s)",
name, indicated_breakpoints)

def remove_unreferenced_events(self):
def remove_unreferenced_events(self) -> None:
"""Remove events from storage that are not referenced.
In older versions of the framework, events that had no observers would get recorded but
Expand Down
6 changes: 3 additions & 3 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ def remove_revision(self, revision: int):
self._id = self.get_info().id
self._backend.secret_remove(typing.cast(str, self.id), revision=revision)

def remove_all_revisions(self):
def remove_all_revisions(self) -> None:
"""Remove all revisions of this secret.
This is called when the secret is no longer needed, for example when
Expand Down Expand Up @@ -1885,11 +1885,11 @@ def can_connect(self) -> bool:
return False
return True

def autostart(self):
def autostart(self) -> None:
"""Autostart all services marked as ``startup: enabled``."""
self._pebble.autostart_services()

def replan(self):
def replan(self) -> None:
"""Replan all services: restart changed services and start startup-enabled services."""
self._pebble.replan_services()

Expand Down
8 changes: 4 additions & 4 deletions ops/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ def _setup(self):
''')
self._db.commit()

def close(self):
def close(self) -> None:
"""Part of the Storage API, close the storage backend."""
self._db.close()

def commit(self):
def commit(self) -> None:
"""Part of the Storage API, commit latest changes in the storage backend."""
self._db.commit()

Expand Down Expand Up @@ -203,13 +203,13 @@ class JujuStorage:
def __init__(self, backend: Optional['_JujuStorageBackend'] = None):
self._backend: _JujuStorageBackend = backend or _JujuStorageBackend()

def close(self):
def close(self) -> None:
"""Part of the Storage API, close the storage backend.
Nothing to be done for Juju backend, as it's transactional.
"""

def commit(self):
def commit(self) -> None:
"""Part of the Storage API, commit latest changes in the storage backend.
Nothing to be done for Juju backend, as it's transactional.
Expand Down
2 changes: 1 addition & 1 deletion ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ def set_planned_units(self, num_units: int) -> None:
raise TypeError("num_units must be 0 or a positive integer.")
self._backend._planned_units = num_units

def reset_planned_units(self):
def reset_planned_units(self) -> None:
"""Reset the planned units override.
This allows the harness to fall through to the built in methods that will try to
Expand Down

0 comments on commit 384ef3a

Please sign in to comment.