Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: run pre-commit on all files #1474

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion HACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
7 changes: 3 additions & 4 deletions ops/_private/harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ops/framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__

Expand Down
10 changes: 5 additions & 5 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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}')


Expand Down
16 changes: 3 additions & 13 deletions ops/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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([
Expand Down
5 changes: 4 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand Down
60 changes: 30 additions & 30 deletions test/charms/test_main/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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__)
Expand Down Expand Up @@ -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__)

Expand Down
2 changes: 1 addition & 1 deletion test/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,7 @@ def test_breakpoint_good_names(self, request: pytest.FixtureRequest, name: str):
'-',
'...foo',
'foo.bar',
'bar--' 'FOO',
'bar--FOO',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this was a bug, perhaps 'bar--', 'FOO' was meant?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, looking at the original commit. Nice find!

'FooBar',
'foo bar',
'foo_bar',
Expand Down
10 changes: 5 additions & 5 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
]
Expand Down Expand Up @@ -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'),
Expand All @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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:
Expand Down
Loading