Skip to content

Commit

Permalink
use pyright instead of mypy (#157)
Browse files Browse the repository at this point in the history
* use pyright instead of mypy

* remove accidental commit

* address PR comments
  • Loading branch information
lucabello authored May 10, 2023
1 parent 44efcb1 commit 9cc768b
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 75 deletions.
10 changes: 5 additions & 5 deletions lib/charms/alertmanager_k8s/v0/alertmanager_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def __init__(self, *args):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 6
LIBPATCH = 7

# Set to match metadata.yaml
INTERFACE_NAME = "alertmanager_dispatch"
Expand Down Expand Up @@ -150,7 +150,7 @@ class AlertmanagerConsumer(RelationManagerBase):
charm (CharmBase): consumer charm
"""

on = AlertmanagerConsumerEvents()
on = AlertmanagerConsumerEvents() # pyright: ignore

def __init__(self, charm: CharmBase, relation_name: str = "alerting"):
super().__init__(charm, relation_name, RelationRole.requires)
Expand All @@ -168,7 +168,7 @@ def _on_relation_changed(self, event: ops.charm.RelationChangedEvent):
"""This hook notifies the charm that there may have been changes to the cluster."""
if event.unit: # event.unit may be `None` in the case of app data change
# inform consumer about the change
self.on.cluster_changed.emit()
self.on.cluster_changed.emit() # pyright: ignore

def get_cluster_info(self) -> List[str]:
"""Returns a list of ip addresses of all the alertmanager units."""
Expand All @@ -184,12 +184,12 @@ def get_cluster_info(self) -> List[str]:

def _on_relation_departed(self, _):
"""This hook notifies the charm that there may have been changes to the cluster."""
self.on.cluster_changed.emit()
self.on.cluster_changed.emit() # pyright: ignore

def _on_relation_broken(self, _):
"""This hook notifies the charm that a relation has been completely removed."""
# inform consumer about the change
self.on.cluster_changed.emit()
self.on.cluster_changed.emit() # pyright: ignore


class AlertmanagerProvider(RelationManagerBase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 2
LIBPATCH = 3

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -137,7 +137,7 @@ def get_config(self, *args):
`alertmanager-k8s` charm, which requires such separation.
"""

on = AlertmanagerRemoteConfigurationRequirerEvents()
on = AlertmanagerRemoteConfigurationRequirerEvents() # pyright: ignore

def __init__(
self,
Expand Down Expand Up @@ -174,7 +174,7 @@ def _on_relation_changed(self, _) -> None:
Emits custom `remote_configuration_changed` event every time remote configuration
changes.
"""
self.on.remote_configuration_changed.emit()
self.on.remote_configuration_changed.emit() # pyright: ignore

def _on_relation_broken(self, _) -> None:
"""Event handler for remote configuration relation broken event.
Expand Down Expand Up @@ -327,7 +327,7 @@ def __init__(self, *args):
`alertmanager-k8s` charm, which requires such separation.
"""

on = AlertmanagerRemoteConfigurationProviderEvents()
on = AlertmanagerRemoteConfigurationProviderEvents() # pyright: ignore

def __init__(
self,
Expand Down Expand Up @@ -423,7 +423,7 @@ def update_relation_data_bag(self, alertmanager_config: Optional[dict]) -> None:
else:
logger.warning("Invalid Alertmanager configuration. Ignoring...")
self._clear_relation_data()
self.on.configuration_broken.emit()
self.on.configuration_broken.emit() # pyright: ignore

def _prepare_relation_data(
self, config: Optional[dict]
Expand Down
34 changes: 4 additions & 30 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,10 @@ per-file-ignores = {"tests/*" = ["D100","D101","D102","D103"]}
convention = "google"

# Static analysis tools configuration
[tool.mypy]
pretty = true
python_version = 3.8
mypy_path = "$MYPY_CONFIG_FILE_DIR/src:$MYPY_CONFIG_FILE_DIR/lib"
follow_imports = "normal"
warn_redundant_casts = true
warn_unused_ignores = false
warn_unused_configs = true
show_traceback = true
show_error_codes = true
namespace_packages = true
explicit_package_bases = true
check_untyped_defs = true
allow_redefinition = true
no_implicit_optional = false

# Ignore libraries that do not have type hint nor stubs
[[tool.mypy.overrides]]
module = ["lightkube.*", "git.*", "validators.*"]
ignore_missing_imports = true

[[tool.mypy.overrides]]
module = ["charms.grafana_k8s.*", "charms.traefik_k8s.*", "charms.observability_libs.*"]
follow_imports = "silent"
warn_unused_ignores = false

[[tool.mypy.overrides]]
module = ["ops.*", "pytest_operator.*"]
follow_imports = "skip"
ignore_missing_imports = true
[tool.pyright]
extraPaths = ["lib"]
pythonVersion = "3.8"
pythonPlatform = "Linux"

[tool.pytest.ini_options]
minversion = "6.0"
Expand Down
2 changes: 1 addition & 1 deletion src/alertmanager_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def __init__(
port: int = 9093,
*,
web_route_prefix: str = "",
timeout=2.0,
timeout=2,
):
if web_route_prefix and not web_route_prefix.endswith("/"):
web_route_prefix += "/"
Expand Down
46 changes: 27 additions & 19 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
Relation,
WaitingStatus,
)
from ops.pebble import ChangeError, ExecError, Layer, PathError, ProtocolError
from ops.pebble import ChangeError, ExecError, Layer, PathError, ProtocolError # type: ignore

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -93,8 +93,8 @@ def __init__(self, *args):
self._stored.set_default(config_hash=None, launched_with_peers=False)

self.ingress = IngressPerAppRequirer(self, port=self.api_port)
self.framework.observe(self.ingress.on.ready, self._handle_ingress)
self.framework.observe(self.ingress.on.revoked, self._handle_ingress)
self.framework.observe(self.ingress.on.ready, self._handle_ingress) # pyright: ignore
self.framework.observe(self.ingress.on.revoked, self._handle_ingress) # pyright: ignore

# The `_external_url` property is passed as a callable so that the charm library code
# always uses up-to-date context.
Expand Down Expand Up @@ -131,7 +131,9 @@ def __init__(self, *args):
self._container_name,
resource_reqs_func=self._resource_reqs_from_config,
)
self.framework.observe(self.resources_patch.on.patch_failed, self._on_k8s_patch_failed)
self.framework.observe(
self.resources_patch.on.patch_failed, self._on_k8s_patch_failed # pyright: ignore
)

# Self-monitoring
self._scraping = MetricsEndpointProvider(
Expand All @@ -140,8 +142,8 @@ def __init__(self, *args):
jobs=self.self_scraping_job,
refresh_event=[
self.on.update_status,
self.ingress.on.ready,
self.ingress.on.revoked,
self.ingress.on.ready, # pyright: ignore
self.ingress.on.revoked, # pyright: ignore
self.on["ingress"].relation_changed,
self.on["ingress"].relation_departed,
],
Expand All @@ -150,8 +152,8 @@ def __init__(self, *args):
self.catalog = CatalogueConsumer(
charm=self,
refresh_event=[
self.ingress.on.ready,
self.ingress.on.revoked,
self.ingress.on.ready, # pyright: ignore
self.ingress.on.revoked, # pyright: ignore
self.on["ingress"].relation_changed,
self.on.update_status,
self.on.config_changed, # web_external_url; also covers upgrade-charm
Expand All @@ -172,13 +174,15 @@ def __init__(self, *args):

# Core lifecycle events
self.framework.observe(self.on.config_changed, self._on_config_changed)
self.framework.observe(self.on.alertmanager_pebble_ready, self._on_pebble_ready)
self.framework.observe(
self.on.alertmanager_pebble_ready, self._on_pebble_ready # pyright: ignore
)
self.framework.observe(self.on.update_status, self._on_update_status)
self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm)

# Remote configuration events
self.framework.observe(
self.remote_configuration.on.remote_configuration_changed,
self.remote_configuration.on.remote_configuration_changed, # pyright: ignore
self._on_remote_configuration_changed,
)

Expand All @@ -191,8 +195,12 @@ def __init__(self, *args):
)

# Action events
self.framework.observe(self.on.show_config_action, self._on_show_config_action)
self.framework.observe(self.on.check_config_action, self._on_check_config)
self.framework.observe(
self.on.show_config_action, self._on_show_config_action # pyright: ignore
)
self.framework.observe(
self.on.check_config_action, self._on_check_config # pyright: ignore
)

@property
def self_scraping_job(self):
Expand All @@ -214,7 +222,7 @@ def _resource_reqs_from_config(self) -> ResourceRequirements:
return adjust_resource_requirements(limits, requests, adhere_to_requests=True)

def _on_k8s_patch_failed(self, event: K8sResourcePatchFailedEvent):
self.unit.status = BlockedStatus(event.message)
self.unit.status = BlockedStatus(str(event.message))

def _handle_ingress(self, _):
if url := self.ingress.url:
Expand All @@ -223,7 +231,7 @@ def _handle_ingress(self, _):
logger.info("Ingress revoked.")
self._common_exit_hook()

def _check_config(self) -> Tuple[str, str]:
def _check_config(self) -> Tuple[Optional[str], Optional[str]]:
container = self.unit.get_container(self._container_name)

if not container.can_connect():
Expand All @@ -234,14 +242,14 @@ def _check_config(self) -> Tuple[str, str]:
except ChangeError as e:
output, err = "", e.err
except ExecError as e:
output, err = e.stdout, e.stderr
output, err = str(e.stdout), str(e.stderr)

return output, err

def _on_check_config(self, event: ActionEvent) -> None:
"""Runs `amtool check-config` inside the workload."""
output, err = self._check_config()
if not output:
if not output and err:
event.fail(err)
return

Expand All @@ -258,7 +266,7 @@ def _on_show_config_action(self, event: ActionEvent):
try:
content = self.container.pull(self._config_path)
# juju requires keys to be lowercase alphanumeric (can't use self._config_path)
event.set_results({"path": self._config_path, "content": content.read()})
event.set_results({"path": self._config_path, "content": str(content.read())})
except (ProtocolError, PathError) as e:
event.fail(str(e))

Expand Down Expand Up @@ -469,7 +477,7 @@ def _update_config(self) -> None:
# Calculate hash of all the contents of the pending files.
config_hash = sha256("".join(config[1] for config in pending))

if config_hash == self._stored.config_hash:
if config_hash == self._stored.config_hash: # pyright: ignore
logger.debug("no change in config")
return

Expand All @@ -494,7 +502,7 @@ def _push_config_and_reload(self, pending_config: List[Tuple[str, str]]): # noq
f"Failed to push config file '{path}' into container: {e}"
)

output, err = self._check_config()
_, err = self._check_config()
if err:
raise ConfigUpdateFailure(
f"Failed to validate config (run check-config action): {err}"
Expand Down
19 changes: 4 additions & 15 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,16 @@ commands =
ruff {[vars]all_path}
black --check --diff {[vars]all_path}

[testenv:static-{charm,lib,unit,integration}]
[testenv:static-{charm,lib}]
description = Run static analysis checks
setenv =
unit: MYPYPATH = {[vars]tst_path}/unit
integration: MYPYPATH = {[vars]tst_path}/integration
deps =
mypy
types-PyYAML
types-requests
types-setuptools
types-toml
pyright
charm: -r{toxinidir}/requirements.txt
lib: ops
unit: {[testenv:unit]deps}
integration: {[testenv:integration]deps}
commands =
charm: mypy {[vars]src_path} {posargs}
lib: mypy --python-version 3.5 {[vars]lib_path} {posargs}
charm: pyright {[vars]src_path} {posargs}
lib: pyright --pythonversion 3.5 {[vars]lib_path} {posargs}
lib: /usr/bin/env sh -c 'for m in $(git diff main --name-only {[vars]lib_path}); do if ! git diff main $m | grep -q "+LIBPATCH\|+LIBAPI"; then echo "You forgot to bump the version on $m!"; exit 1; fi; done'
unit: mypy {[vars]tst_path}/unit {posargs}
integration: mypy {[vars]tst_path}/integration {posargs}
allowlist_externals = /usr/bin/env

[testenv:reqs]
Expand Down

0 comments on commit 9cc768b

Please sign in to comment.