From b1ed62ec24c00db8fae2457bab57b7de5bca3f1c Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Wed, 25 Jan 2023 09:10:45 +1300 Subject: [PATCH] A few minor tweaks now that we're on Python 3.8 (#890) Originally this PR was running `pyupgrade --py38-plus` on the codebase, to pick up a few new features in Python 3.7 and 3.8 (apart from f-strings, which was done in https://github.com/canonical/operator/pull/889). However, in particular we didn't like how it converted `TypedDict` to the class syntax, because then half of them were converted (the ones without `-` in the field names can't be converted). So avoid that so they're all consistent. This includes: * Use of set literals and comprehensions instead of set(...) * Use OSError instead of IOError as the latter is now an alias * Use b'' instead of bytes() * One use of f-string that apparently flynt didn't pick up This PR also: * Removes the tox 4.2.8 pinning now that https://github.com/tox-dev/tox/issues/2863 is fixed. * Makes tox.ini source vars relative to fix autopep8 problem --- .github/workflows/framework-tests.yaml | 8 ++++---- .github/workflows/observability-charm-tests.yaml | 2 +- ops/_private/yaml.py | 6 +++--- ops/charm.py | 2 +- ops/framework.py | 9 +++++---- ops/pebble.py | 2 +- pyproject.toml | 6 +++--- test/pebble_cli.py | 2 +- test/test_framework.py | 4 ++-- test/test_pebble.py | 2 +- tox.ini | 6 ++++-- 11 files changed, 26 insertions(+), 23 deletions(-) diff --git a/.github/workflows/framework-tests.yaml b/.github/workflows/framework-tests.yaml index 1e0bdbc51..9d5e6ef08 100644 --- a/.github/workflows/framework-tests.yaml +++ b/.github/workflows/framework-tests.yaml @@ -12,7 +12,7 @@ jobs: uses: actions/setup-python@v2 - name: Install tox - run: pip install tox==4.2.8 + run: pip install tox - name: Run linting run: tox -e lint @@ -26,7 +26,7 @@ jobs: uses: actions/setup-python@v2 - name: Install tox - run: pip install tox==4.2.8 + run: pip install tox - name: Run static type checks run: tox -e static @@ -47,7 +47,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install tox - run: pip install tox==4.2.8 + run: pip install tox - name: Run unit tests run: tox -e unit @@ -73,7 +73,7 @@ jobs: go-version: "1.17" - name: Install tox - run: pip install tox==4.2.8 + run: pip install tox - name: Install Pebble run: go install github.com/canonical/pebble/cmd/pebble@latest diff --git a/.github/workflows/observability-charm-tests.yaml b/.github/workflows/observability-charm-tests.yaml index dd34ec528..6625ea3ec 100644 --- a/.github/workflows/observability-charm-tests.yaml +++ b/.github/workflows/observability-charm-tests.yaml @@ -25,7 +25,7 @@ jobs: echo -e "\ngit+$GITHUB_SERVER_URL/$GITHUB_REPOSITORY@$GITHUB_SHA#egg=ops" >> requirements.txt - name: Install dependencies - run: pip install tox==4.2.8 + run: pip install tox - name: Run the charm tests run: tox -vve unit diff --git a/ops/_private/yaml.py b/ops/_private/yaml.py index 8729d5ce4..94c1fd7b4 100644 --- a/ops/_private/yaml.py +++ b/ops/_private/yaml.py @@ -23,9 +23,9 @@ _safe_dumper = getattr(yaml, 'CSafeDumper', yaml.SafeDumper) -def safe_load(stream: Union[str, TextIO]): +def safe_load(stream: Union[str, TextIO]) -> Any: """Same as yaml.safe_load, but use fast C loader if available.""" - return yaml.load(stream, Loader=_safe_loader) + return yaml.load(stream, Loader=_safe_loader) # type: ignore @overload @@ -40,4 +40,4 @@ def safe_dump(data: Any, stream: Optional[Union[str, TextIO]] = None, **kwargs: If `encoding:str` is provided, return bytes. Else, return str. """ - return yaml.dump(data, stream=stream, Dumper=_safe_dumper, **kwargs) + return yaml.dump(data, stream=stream, Dumper=_safe_dumper, **kwargs) # type: ignore diff --git a/ops/charm.py b/ops/charm.py index 752c43cca..986e4b854 100755 --- a/ops/charm.py +++ b/ops/charm.py @@ -1195,7 +1195,7 @@ def __init__(self, role: RelationRole, relation_name: str, raw: '_RelationMetaDi self.scope = raw.get('scope') or self._default_scope if self.scope not in self.VALID_SCOPES: raise TypeError("scope should be one of {}; not '{}'".format( - ', '.join("'{}'".format(s) for s in self.VALID_SCOPES), self.scope)) + ', '.join(f"'{s}'" for s in self.VALID_SCOPES), self.scope)) class StorageMeta: diff --git a/ops/framework.py b/ops/framework.py index 3057cd081..0ba99e156 100755 --- a/ops/framework.py +++ b/ops/framework.py @@ -50,8 +50,7 @@ if TYPE_CHECKING: from pathlib import Path - - from typing_extensions import Literal, Protocol, Type + from typing import Literal, Protocol, Type from ops.charm import CharmMeta from ops.model import JsonObject, Model, _ModelBackend @@ -613,8 +612,10 @@ def __init__(self, storage: Union[SQLiteStorage, JujuStorage], # Parse the env var once, which may be used multiple times later debug_at = os.environ.get('JUJU_DEBUG_AT') - self._juju_debug_at = (set(x.strip() for x in debug_at.split(',')) - if debug_at else set()) # type: Set[str] + if debug_at: + self._juju_debug_at = {x.strip() for x in debug_at.split(',')} + else: + self._juju_debug_at: Set[str] = set() def set_breakpointhook(self): """Hook into sys.breakpointhook so the builtin breakpoint() works as expected. diff --git a/ops/pebble.py b/ops/pebble.py index d0352ddfd..c129f8b0c 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -1486,7 +1486,7 @@ def _request_raw( try: body = json.loads(e.read()) # type: Dict[str, Any] message = body['result']['message'] # type: str - except (IOError, ValueError, KeyError) as e2: + except (OSError, ValueError, KeyError) as e2: # Will only happen on read error or if Pebble sends invalid JSON. body = {} # type: Dict[str, Any] message = f'{type(e2).__name__} - {e2}' diff --git a/pyproject.toml b/pyproject.toml index 2d2b20744..6f43dc4b1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,12 +28,12 @@ per-file-ignores = ["test/*:D100,D101,D102,D103,D104"] docstring-convention = "google" [tool.pyright] -include = ["ops/jujuversion.py", "ops/log.py", "ops/model.py", "ops/version.py", - "ops/__init__.py", "ops/framework.py", "ops/pebble.py", "ops/charm.py", - "ops/storage.py", "ops/main.py", "ops/testing.py", "ops/_private/timeconv.py"] +include = ["ops/*.py", "ops/_private/*.py"] pythonVersion = "3.8" # check no python > 3.8 features are used pythonPlatform = "All" typeCheckingMode = "strict" reportIncompatibleMethodOverride = false reportImportCycles = false reportTypeCommentUsage = false +reportMissingModuleSource = false +stubPath = "" diff --git a/test/pebble_cli.py b/test/pebble_cli.py index 21c906902..05fc42265 100644 --- a/test/pebble_cli.py +++ b/test/pebble_cli.py @@ -151,7 +151,7 @@ def main(): try: with open(args.layer_path, encoding='utf-8') as f: layer_yaml = f.read() - except IOError as e: + except OSError as e: parser.error(f'cannot read layer YAML: {e}') client.add_layer(args.label, layer_yaml, combine=bool(args.combine)) result = f'Layer {args.label!r} added successfully from {args.layer_path!r}' diff --git a/test/test_framework.py b/test/test_framework.py index 57b5c9319..d0267ab7c 100755 --- a/test/test_framework.py +++ b/test/test_framework.py @@ -1233,11 +1233,11 @@ def test_mutable_types(self): ), ( lambda: set(), 'a', - set(['a']), + {'a'}, lambda a, b: a.add(b), lambda res, expected_res: self.assertEqual(res, expected_res) ), ( - lambda: set(['a']), + lambda: {'a'}, 'a', set(), lambda a, b: a.discard(b), diff --git a/test/test_pebble.py b/test/test_pebble.py index d7af531f8..5cfb0929d 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -1178,7 +1178,7 @@ def test_multipart_parser(self): def handle_header(data): headers.append(bytes(data)) - bodies.append(bytes()) + bodies.append(b'') bodies_done.append(False) def handle_body(data, done=False): diff --git a/tox.ini b/tox.ini index 542a24ad7..733ad7908 100644 --- a/tox.ini +++ b/tox.ini @@ -6,8 +6,10 @@ skip_missing_interpreters = True envlist = lint, unit [vars] -src_path = {toxinidir}/ops/ -tst_path = {toxinidir}/test/ +# These need to be relative paths because autopep8 doesn't handle absolute +# paths with relative paths in "exclude" correctly. +src_path = ops/ +tst_path = test/ all_path = {[vars]src_path} {[vars]tst_path} [testenv]