chore: align pre-commit with tox/lint

Author: dimaqq

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.5.0
+    rev: v5.0.0
     hooks:
       - id: check-added-large-files
       - id: check-ast
@@ -15,18 +15,19 @@ repos:
       - id: mixed-line-ending
       - id: end-of-file-fixer
       - id: trailing-whitespace
+        exclude: "test_pebble[.]py$"
       - id: detect-private-key
   # Run the Ruff linter.
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.4.5
+    rev: v0.8.0
     hooks:
       - id: ruff
         args: [ --preview ]
       - id: ruff-format
         args: [ --preview ]
   # Spellcheck the code.
   - repo: https://github.com/codespell-project/codespell
-    rev: v2.2.4
+    rev: v2.3.0
     hooks:
     - id: codespell
       additional_dependencies:

HACKING.md

@@ -466,7 +466,7 @@ The main improvements in this release are ...
 Read more in the [full release notes on GitHub](link to the GitHub release).
 ```
 
-In the post, outline the key improvements both in `ops` and `ops-scenario` - 
+In the post, outline the key improvements both in `ops` and `ops-scenario` -
 the point here is to encourage people to check out the full notes and to upgrade
 promptly, so ensure that you entice them with the best that the new versions
 have to offer.

ops/_private/harness.py

@@ -159,7 +159,7 @@ class ExecResult:
     stderr: Union[str, bytes] = b''
 
 
-ExecHandler = Callable[[ExecArgs], Union[None, ExecResult]]
+ExecHandler = Callable[[ExecArgs], Union[ExecResult, None]]
 
 
 @dataclasses.dataclass(frozen=True)
@@ -1789,8 +1789,7 @@ def revoke_secret(self, secret_id: str, observer: AppUnitOrName):
         # Model secrets:
         if secret.owner_name in [self.model.app.name, self.model.unit.name]:
             raise RuntimeError(
-                f'Secret {secret_id!r} owned by the charm under test, "'
-                f"can't call revoke_secret"
+                f'Secret {secret_id!r} owned by the charm under test, "can\'t call revoke_secret'
             )
 
         relation_id = self._secret_relation_id_to(secret)
@@ -2113,7 +2112,7 @@ def run_action(
                         f'given {{"{key}":{params[key]!r}}}'
                     )
         action_under_test = _RunningAction(action_name, ActionOutput([], {}), params)
-        handler = getattr(self.charm.on, f"{action_name.replace('-', '_')}_action")
+        handler = getattr(self.charm.on, f'{action_name.replace("-", "_")}_action')
         self._backend._running_action = action_under_test
         self._action_id_counter += 1
         handler.emit(str(self._action_id_counter))

ops/charm.py

@@ -1682,9 +1682,9 @@ class RelationMeta:
     VALID_SCOPES: ClassVar[List[str]] = ['global', 'container']
 
     def __init__(self, role: RelationRole, relation_name: str, raw: '_RelationMetaDict'):
-        assert isinstance(
-            role, RelationRole
-        ), f'role should be one of {list(RelationRole)!r}, not {role!r}'
+        assert isinstance(role, RelationRole), (
+            f'role should be one of {list(RelationRole)!r}, not {role!r}'
+        )
         self._default_scope = self.VALID_SCOPES[0]
         self.role = role
         self.relation_name = relation_name

ops/framework.py

@@ -805,7 +805,7 @@ class SomeObject:
         method_name = observer.__name__
 
         assert isinstance(observer.__self__, Object), (
-            "can't register observers " "that aren't `Object`s"
+            "can't register observers that aren't `Object`s"
         )
         observer_obj = observer.__self__
 

ops/model.py

@@ -1313,7 +1313,7 @@ def __repr__(self):
             fields.append(f'id={self._id!r}')
         if self._label is not None:
             fields.append(f'label={self._label!r}')
-        return f"<Secret {' '.join(fields)}>"
+        return f'<Secret {" ".join(fields)}>'
 
     @staticmethod
     def _canonicalize_id(id: str, model_uuid: Optional[str]) -> str:
@@ -2218,7 +2218,7 @@ def request(self, storage_name: str, count: int = 1):
         """
         if storage_name not in self._storage_map:
             raise ModelError(
-                f'cannot add storage {storage_name!r}:' ' it is not present in the charm metadata'
+                f'cannot add storage {storage_name!r}: it is not present in the charm metadata'
             )
         self._backend.storage_add(storage_name, count)
 
@@ -3879,12 +3879,12 @@ def validate_metric_label(cls, label_name: str):
     def format_metric_value(cls, value: Union[int, float]):
         if not isinstance(value, (int, float)):
             raise ModelError(
-                f'invalid metric value {value!r} provided:' ' must be a positive finite float'
+                f'invalid metric value {value!r} provided: must be a positive finite float'
             )
 
         if math.isnan(value) or math.isinf(value) or value < 0:
             raise ModelError(
-                f'invalid metric value {value!r} provided:' ' must be a positive finite float'
+                f'invalid metric value {value!r} provided: must be a positive finite float'
             )
         return str(value)
 
@@ -3895,7 +3895,7 @@ def validate_label_value(cls, label: str, value: str):
         if not value:
             raise ModelError(f'metric label {label} has an empty value, which is not allowed')
         v = str(value)
-        if re.search('[,=]', v) is not None:
+        if re.search(r'[,=]', v) is not None:
             raise ModelError(f'metric label values must not contain "," or "=": {label}={value!r}')
 
 

ops/pebble.py

@@ -661,12 +661,7 @@ def from_dict(cls, d: _ProgressDict) -> TaskProgress:
         )
 
     def __repr__(self):
-        return (
-            'TaskProgress('
-            f'label={self.label!r}, '
-            f'done={self.done!r}, '
-            f'total={self.total!r})'
-        )
+        return f'TaskProgress(label={self.label!r}, done={self.done!r}, total={self.total!r})'
 
 
 class TaskID(str):
@@ -1083,12 +1078,7 @@ def from_dict(cls, d: _ServiceInfoDict) -> ServiceInfo:
         )
 
     def __repr__(self):
-        return (
-            'ServiceInfo('
-            f'name={self.name!r}, '
-            f'startup={self.startup}, '
-            f'current={self.current})'
-        )
+        return f'ServiceInfo(name={self.name!r}, startup={self.startup}, current={self.current})'
 
 
 class Check:
@@ -2547,7 +2537,7 @@ def _encode_multipart(
             source_io: _AnyStrFileLikeIO = source  # type: ignore
         boundary = binascii.hexlify(os.urandom(16))
         path_escaped = path.replace('"', '\\"').encode('utf-8')
-        content_type = f"multipart/form-data; boundary=\"{boundary.decode('utf-8')}\""
+        content_type = f'multipart/form-data; boundary="{boundary.decode("utf-8")}"'
 
         def generator() -> Generator[bytes, None, None]:
             yield b''.join([

pyproject.toml

@@ -177,7 +177,10 @@ keep-runtime-typing = true
     "S106",
 
     # "Useless" expression.
-    "B018"
+    "B018",
+
+    # Ruff prefers regular expressions as raw strings.
+    "RUF039",
 ]
 "ops/_private/timeconv.py" = [
     "RUF001",  # String contains ambiguous `µ` (MICRO SIGN). Did you mean `μ` (GREEK SMALL LETTER MU)?

test/charms/test_main/src/charm.py

@@ -153,9 +153,9 @@ def _on_db_relation_joined(self, event: ops.RelationJoinedEvent):
         self._stored.db_relation_joined_data = event.snapshot()
 
     def _on_mon_relation_changed(self, event: ops.RelationChangedEvent):
-        assert (
-            event.app is not None
-        ), 'application name cannot be None for a relation-changed event'
+        assert event.app is not None, (
+            'application name cannot be None for a relation-changed event'
+        )
         if os.environ.get('JUJU_REMOTE_UNIT'):
             assert event.unit is not None, (
                 'a unit name cannot be None for a relation-changed event'
@@ -168,22 +168,22 @@ def _on_mon_relation_changed(self, event: ops.RelationChangedEvent):
         self._stored.mon_relation_changed_data = event.snapshot()
 
     def _on_mon_relation_departed(self, event: ops.RelationDepartedEvent):
-        assert (
-            event.app is not None
-        ), 'application name cannot be None for a relation-departed event'
+        assert event.app is not None, (
+            'application name cannot be None for a relation-departed event'
+        )
         assert event.relation.active, 'a departed relation is still active'
         assert self.model.relations['mon']
         self._stored.on_mon_relation_departed.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
         self._stored.mon_relation_departed_data = event.snapshot()
 
     def _on_ha_relation_broken(self, event: ops.RelationBrokenEvent):
-        assert (
-            event.app is None
-        ), 'relation-broken events cannot have a reference to a remote application'
-        assert (
-            event.unit is None
-        ), 'relation broken events cannot have a reference to a remote unit'
+        assert event.app is None, (
+            'relation-broken events cannot have a reference to a remote application'
+        )
+        assert event.unit is None, (
+            'relation broken events cannot have a reference to a remote unit'
+        )
         assert not event.relation.active, 'relation broken events always have a broken relation'
         assert not self.model.relations['ha']
         self._stored.on_ha_relation_broken.append(type(event).__name__)
@@ -218,56 +218,56 @@ def _on_test_pebble_check_recovered(self, event: ops.PebbleCheckRecoveredEvent):
         self._stored.test_pebble_check_recovered_data = event.snapshot()
 
     def _on_start_action(self, event: ops.ActionEvent):
-        assert (
-            event.handle.kind == 'start_action'
-        ), 'event action name cannot be different from the one being handled'
+        assert event.handle.kind == 'start_action', (
+            'event action name cannot be different from the one being handled'
+        )
         self._stored.on_start_action.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
 
     def _on_secret_changed(self, event: ops.SecretChangedEvent):
         # subprocess and isinstance don't mix well
-        assert (
-            type(event.secret).__name__ == 'Secret'
-        ), f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        assert type(event.secret).__name__ == 'Secret', (
+            f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        )
         assert event.secret.id, 'secret must have an ID'
         self._stored.on_secret_changed.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
         self._stored.secret_changed_data = event.snapshot()
 
     def _on_secret_remove(self, event: ops.SecretRemoveEvent):
         # subprocess and isinstance don't mix well
-        assert (
-            type(event.secret).__name__ == 'Secret'
-        ), f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        assert type(event.secret).__name__ == 'Secret', (
+            f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        )
         assert event.secret.id, 'secret must have an ID'
         self._stored.on_secret_remove.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
         self._stored.secret_remove_data = event.snapshot()
 
     def _on_secret_rotate(self, event: ops.SecretRotateEvent):
         # subprocess and isinstance don't mix well
-        assert (
-            type(event.secret).__name__ == 'Secret'
-        ), f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        assert type(event.secret).__name__ == 'Secret', (
+            f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        )
         assert event.secret.id, 'secret must have an ID'
         self._stored.on_secret_rotate.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
         self._stored.secret_rotate_data = event.snapshot()
 
     def _on_secret_expired(self, event: ops.SecretExpiredEvent):
         # subprocess and isinstance don't mix well
-        assert (
-            type(event.secret).__name__ == 'Secret'
-        ), f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        assert type(event.secret).__name__ == 'Secret', (
+            f'SecretEvent.secret must be a Secret instance, not {type(event.secret)}'
+        )
         assert event.secret.id, 'secret must have an ID'
         self._stored.on_secret_expired.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
         self._stored.secret_expired_data = event.snapshot()
 
     def _on_foo_bar_action(self, event: ops.ActionEvent):
-        assert (
-            event.handle.kind == 'foo_bar_action'
-        ), 'event action name cannot be different from the one being handled'
+        assert event.handle.kind == 'foo_bar_action', (
+            'event action name cannot be different from the one being handled'
+        )
         self._stored.on_foo_bar_action.append(type(event).__name__)
         self._stored.observed_event_types.append(type(event).__name__)
 

test/test_framework.py

@@ -1810,7 +1810,7 @@ def test_breakpoint_good_names(self, request: pytest.FixtureRequest, name: str):
             '-',
             '...foo',
             'foo.bar',
-            'bar--' 'FOO',
+            'bar--FOO',
             'FooBar',
             'foo bar',
             'foo_bar',

test/test_main.py

@@ -1046,7 +1046,7 @@ def test_setup_event_links(
     ):
         """Test auto-creation of symlinks caused by initial events."""
         all_event_hooks = [
-            f"hooks/{name.replace('_', '-')}"
+            f'hooks/{name.replace("_", "-")}'
             for name, event_source in self.charm_module.Charm.on.events().items()
             if issubclass(event_source.event_type, (ops.CommitEvent, ops.PreCommitEvent))
         ]
@@ -1138,7 +1138,7 @@ def test_setup_event_links(
         Symlink creation caused by initial events should _not_ happen when using dispatch.
         """
         all_event_hooks = [
-            f"hooks/{e.replace('_', '-')}" for e in self.charm_module.Charm.on.events()
+            f'hooks/{e.replace("_", "-")}' for e in self.charm_module.Charm.on.events()
         ]
         initial_events = {
             EventSpec(ops.InstallEvent, 'install'),
@@ -1150,9 +1150,9 @@ def test_setup_event_links(
         def _assess_event_links(event_spec: EventSpec):
             assert self.hooks_dir / event_spec.event_name not in self.hooks_dir.iterdir()
             for event_hook in all_event_hooks:
-                assert not (
-                    self.JUJU_CHARM_DIR / event_hook
-                ).exists(), f'Spurious hook: {event_hook}'
+                assert not (self.JUJU_CHARM_DIR / event_hook).exists(), (
+                    f'Spurious hook: {event_hook}'
+                )
 
         for initial_event in initial_events:
             self._setup_charm_dir(request)

test/test_model.py

@@ -1397,9 +1397,9 @@ def test_recursive_push_and_pull(case: PushPullCase):
             raise
         errors = {src[len(push_src.name) :] for src, _ in err.errors}
 
-    assert (
-        case.errors == errors
-    ), f'push_path gave wrong expected errors: want {case.errors}, got {errors}'
+    assert case.errors == errors, (
+        f'push_path gave wrong expected errors: want {case.errors}, got {errors}'
+    )
     for fpath in case.want:
         assert c.exists(fpath), f'push_path failed: file {fpath} missing at destination'
         content = c.pull(fpath, encoding=None).read()
@@ -1424,9 +1424,9 @@ def test_recursive_push_and_pull(case: PushPullCase):
             raise
         errors = {src for src, _ in err.errors}
 
-    assert (
-        case.errors == errors
-    ), f'pull_path gave wrong expected errors: want {case.errors}, got {errors}'
+    assert case.errors == errors, (
+        f'pull_path gave wrong expected errors: want {case.errors}, got {errors}'
+    )
     for fpath in case.want:
         assert c.exists(fpath), f'pull_path failed: file {fpath} missing at destination'
     for fdir in case.want_dirs: