From 9f0a2f8a0ce17a7501b812e1880da3a1afffcf78 Mon Sep 17 00:00:00 2001 From: LeaveMyYard <33721692+LeaveMyYard@users.noreply.github.com> Date: Wed, 5 Jul 2023 10:54:06 +0300 Subject: Fix linting errors --- robusta_krr/core/integrations/prometheus/loader.py | 13 ++++++++----- .../core/integrations/prometheus/metrics/base_metric.py | 10 +++++++--- .../integrations/prometheus/metrics/memory_metric.py | 5 +++-- .../prometheus/metrics_service/base_metric_service.py | 3 +-- .../metrics_service/prometheus_metrics_service.py | 17 +++++++++-------- .../metrics_service/thanos_metrics_service.py | 5 ++--- .../metrics_service/victoria_metrics_service.py | 5 ++--- robusta_krr/core/runner.py | 4 ++-- robusta_krr/formatters/table.py | 2 +- robusta_krr/utils/configurable.py | 4 ++-- robusta_krr/utils/service_discovery.py | 7 +++++++ 11 files changed, 44 insertions(+), 31 deletions(-) (limited to 'robusta_krr') diff --git a/robusta_krr/core/integrations/prometheus/loader.py b/robusta_krr/core/integrations/prometheus/loader.py index 143fb63..4f3b832 100644 --- a/robusta_krr/core/integrations/prometheus/loader.py +++ b/robusta_krr/core/integrations/prometheus/loader.py @@ -1,6 +1,5 @@ -import asyncio import datetime -from typing import Optional, no_type_check +from typing import Optional from concurrent.futures import ThreadPoolExecutor @@ -49,10 +48,12 @@ class MetricsLoader(Configurable): if cluster is not None else None ) - self.loader = self.get_metrics_service(config, api_client=self.api_client, cluster=cluster) - if not self.loader: + loader = self.get_metrics_service(config, api_client=self.api_client, cluster=cluster) + if loader is None: raise PrometheusNotFound("No Prometheus or metrics service found") + self.loader = loader + self.info(f"{self.loader.name()} connected successfully for {cluster or 'default'} cluster") def get_metrics_service( @@ -68,9 +69,11 @@ class MetricsLoader(Configurable): self.echo(f"{service_name} found") loader.validate_cluster_name() return loader - except MetricsNotFound as e: + except MetricsNotFound: self.debug(f"{service_name} not found") + return None + async def gather_data( self, object: K8sObjectData, diff --git a/robusta_krr/core/integrations/prometheus/metrics/base_metric.py b/robusta_krr/core/integrations/prometheus/metrics/base_metric.py index 8bf263c..4e872b0 100644 --- a/robusta_krr/core/integrations/prometheus/metrics/base_metric.py +++ b/robusta_krr/core/integrations/prometheus/metrics/base_metric.py @@ -15,7 +15,8 @@ from robusta_krr.utils.configurable import Configurable if TYPE_CHECKING: from .. import CustomPrometheusConnect - MetricsDictionary = dict[str, type[BaseMetricLoader]] + +MetricsDictionary = dict[str, type["BaseMetricLoader"]] class QueryType(str, enum.Enum): @@ -72,6 +73,9 @@ class BaseMetricLoader(Configurable, abc.ABC): pass + def get_query_type(self) -> QueryType: + return QueryType.QueryRange + def get_graph_query(self, object: K8sObjectData, resolution: Optional[str]) -> str: """ This method should be implemented by all subclasses to provide a query string in the metadata to produce relevant graphs. @@ -158,7 +162,7 @@ class BaseMetricLoader(Configurable, abc.ABC): step=self._step_to_string(step), ) result = await self.query_prometheus(metric=metric, query_type=query_type) - # adding the query in the results for a graph + # adding the query in the results for a graph metric.query = self.get_graph_query(object, resolution) if result == []: @@ -173,7 +177,7 @@ class BaseMetricLoader(Configurable, abc.ABC): ) @staticmethod - def get_by_resource(resource: str, strategy: Optional[str]) -> type[BaseMetricLoader]: + def get_by_resource(resource: str, strategy: str) -> type[BaseMetricLoader]: """ Fetches the metric loader corresponding to the specified resource. diff --git a/robusta_krr/core/integrations/prometheus/metrics/memory_metric.py b/robusta_krr/core/integrations/prometheus/metrics/memory_metric.py index a8a4521..b87c5c3 100644 --- a/robusta_krr/core/integrations/prometheus/metrics/memory_metric.py +++ b/robusta_krr/core/integrations/prometheus/metrics/memory_metric.py @@ -23,9 +23,10 @@ class MemoryMetricLoader(BaseFilteredMetricLoader): def get_query_type(self) -> QueryType: return QueryType.QueryRange + # This is a temporary solutions, metric loaders will be moved to strategy in the future @override_metric("simple", ResourceType.Memory) -class MemoryMetricLoader(MemoryMetricLoader): +class SimpleMemoryMetricLoader(MemoryMetricLoader): """ A class that overrides the memory metric on the simple strategy. """ @@ -47,5 +48,5 @@ class MemoryMetricLoader(MemoryMetricLoader): def get_query_type(self) -> QueryType: return QueryType.Query - def get_graph_query(self, object: K8sObjectData, resolution: Optional[str]) -> str: + def get_graph_query(self, object: K8sObjectData, resolution: Optional[str]) -> str: return super().get_query(object, resolution) diff --git a/robusta_krr/core/integrations/prometheus/metrics_service/base_metric_service.py b/robusta_krr/core/integrations/prometheus/metrics_service/base_metric_service.py index 4337e5e..3178a61 100644 --- a/robusta_krr/core/integrations/prometheus/metrics_service/base_metric_service.py +++ b/robusta_krr/core/integrations/prometheus/metrics_service/base_metric_service.py @@ -42,7 +42,7 @@ class MetricsService(Configurable, abc.ABC): return classname.replace("MetricsService", "") if classname != MetricsService.__name__ else classname @abc.abstractmethod - async def get_cluster_names(self) -> Optional[List[str]]: + def get_cluster_names(self) -> Optional[List[str]]: ... @abc.abstractmethod @@ -51,7 +51,6 @@ class MetricsService(Configurable, abc.ABC): object: K8sObjectData, resource: ResourceType, period: datetime.timedelta, - *, step: datetime.timedelta = datetime.timedelta(minutes=30), ) -> ResourceHistoryData: ... diff --git a/robusta_krr/core/integrations/prometheus/metrics_service/prometheus_metrics_service.py b/robusta_krr/core/integrations/prometheus/metrics_service/prometheus_metrics_service.py index 31fe0c0..8703e56 100644 --- a/robusta_krr/core/integrations/prometheus/metrics_service/prometheus_metrics_service.py +++ b/robusta_krr/core/integrations/prometheus/metrics_service/prometheus_metrics_service.py @@ -1,7 +1,7 @@ import asyncio from concurrent.futures import ThreadPoolExecutor import datetime -from typing import List, Optional, no_type_check, Type +from typing import Optional, no_type_check, Type import requests from kubernetes.client import ApiClient @@ -13,14 +13,13 @@ from robusta_krr.core.abstract.strategies import ResourceHistoryData from robusta_krr.core.models.config import Config from robusta_krr.core.models.objects import K8sObjectData, PodData from robusta_krr.core.models.result import ResourceType -from robusta_krr.utils.configurable import Configurable -from robusta_krr.utils.service_discovery import ServiceDiscovery +from robusta_krr.utils.service_discovery import MetricsServiceDiscovery from ..metrics import BaseMetricLoader from .base_metric_service import MetricsNotFound, MetricsService -class PrometheusDiscovery(ServiceDiscovery): +class PrometheusDiscovery(MetricsServiceDiscovery): def find_metrics_url(self, *, api_client: Optional[ApiClient] = None) -> Optional[str]: """ Finds the Prometheus URL using selectors. @@ -89,7 +88,7 @@ class PrometheusMetricsService(MetricsService): *, cluster: Optional[str] = None, api_client: Optional[ApiClient] = None, - service_discovery: Type[ServiceDiscovery] = PrometheusDiscovery, + service_discovery: Type[MetricsServiceDiscovery] = PrometheusDiscovery, executor: Optional[ThreadPoolExecutor] = None, ) -> None: super().__init__(config=config, api_client=api_client, cluster=cluster, executor=executor) @@ -116,7 +115,7 @@ class PrometheusMetricsService(MetricsService): if self.auth_header: headers = {"Authorization": self.auth_header} - elif not self.config.inside_cluster: + elif not self.config.inside_cluster and self.api_client is not None: self.api_client.update_params_for_auth(headers, {}, ["BearerToken"]) self.prometheus = CustomPrometheusConnect(url=self.url, disable_ssl=not self.ssl_enabled, headers=headers) @@ -162,7 +161,9 @@ class PrometheusMetricsService(MetricsService): f"Label {cluster_label} does not exist, Rerun krr with the flag `-l ` where is one of {cluster_names}" ) - def get_cluster_names(self) -> Optional[List[str]]: + # Superclass method returns Optional[list[str]], but here we return list[str] + # NOTE that this does not break Liskov Substitution Principle + def get_cluster_names(self) -> list[str]: return self.prometheus.get_label_values(label_name=self.config.prometheus_label) async def gather_data( @@ -179,7 +180,7 @@ class PrometheusMetricsService(MetricsService): MetricLoaderType = BaseMetricLoader.get_by_resource(resource, self.config.strategy) await self.add_historic_pods(object, period) - + metric_loader = MetricLoaderType(self.config, self.prometheus, self.executor) return await metric_loader.load_data(object, period, step, self.name()) diff --git a/robusta_krr/core/integrations/prometheus/metrics_service/thanos_metrics_service.py b/robusta_krr/core/integrations/prometheus/metrics_service/thanos_metrics_service.py index 42066ae..120c471 100644 --- a/robusta_krr/core/integrations/prometheus/metrics_service/thanos_metrics_service.py +++ b/robusta_krr/core/integrations/prometheus/metrics_service/thanos_metrics_service.py @@ -1,15 +1,14 @@ from typing import Optional from kubernetes.client import ApiClient -from requests.exceptions import ConnectionError, HTTPError from concurrent.futures import ThreadPoolExecutor from robusta_krr.core.models.config import Config -from robusta_krr.utils.service_discovery import ServiceDiscovery +from robusta_krr.utils.service_discovery import MetricsServiceDiscovery from .prometheus_metrics_service import MetricsNotFound, PrometheusMetricsService -class ThanosMetricsDiscovery(ServiceDiscovery): +class ThanosMetricsDiscovery(MetricsServiceDiscovery): def find_metrics_url(self, *, api_client: Optional[ApiClient] = None) -> Optional[str]: """ Finds the Thanos URL using selectors. diff --git a/robusta_krr/core/integrations/prometheus/metrics_service/victoria_metrics_service.py b/robusta_krr/core/integrations/prometheus/metrics_service/victoria_metrics_service.py index 9c15cab..38d376c 100644 --- a/robusta_krr/core/integrations/prometheus/metrics_service/victoria_metrics_service.py +++ b/robusta_krr/core/integrations/prometheus/metrics_service/victoria_metrics_service.py @@ -1,15 +1,14 @@ from typing import Optional from concurrent.futures import ThreadPoolExecutor from kubernetes.client import ApiClient -from requests.exceptions import ConnectionError, HTTPError from robusta_krr.core.models.config import Config -from robusta_krr.utils.service_discovery import ServiceDiscovery +from robusta_krr.utils.service_discovery import MetricsServiceDiscovery from .prometheus_metrics_service import MetricsNotFound, PrometheusMetricsService -class VictoriaMetricsDiscovery(ServiceDiscovery): +class VictoriaMetricsDiscovery(MetricsServiceDiscovery): def find_metrics_url(self, *, api_client: Optional[ApiClient] = None) -> Optional[str]: """ Finds the Victoria Metrics URL using selectors. diff --git a/robusta_krr/core/runner.py b/robusta_krr/core/runner.py index 3cdf7b9..1ed4ac5 100644 --- a/robusta_krr/core/runner.py +++ b/robusta_krr/core/runner.py @@ -65,7 +65,7 @@ class Runner(Configurable): Formatter = self.config.Formatter formatted = result.format(Formatter) self.echo("\n", no_prefix=True) - self.print_result(formatted, rich=Formatter.__rich_console__) + self.print_result(formatted, rich=getattr(Formatter, "__rich_console__", False)) def __get_resource_minimal(self, resource: ResourceType) -> float: if resource == ResourceType.CPU: @@ -204,5 +204,5 @@ class Runner(Configurable): self._process_result(result) except ClusterNotSpecifiedException as e: self.error(e) - except Exception as e: + except Exception: self.console.print_exception(extra_lines=1, max_frames=10) diff --git a/robusta_krr/formatters/table.py b/robusta_krr/formatters/table.py index 846732a..a1c6e42 100644 --- a/robusta_krr/formatters/table.py +++ b/robusta_krr/formatters/table.py @@ -1,5 +1,5 @@ import itertools -from typing import Any, Optional +from typing import Any from rich.table import Table diff --git a/robusta_krr/utils/configurable.py b/robusta_krr/utils/configurable.py index 8954139..00c3a64 100644 --- a/robusta_krr/utils/configurable.py +++ b/robusta_krr/utils/configurable.py @@ -93,9 +93,9 @@ class Configurable(abc.ABC): self.echo(message, type="WARNING") - def error(self, message: str = "") -> None: + def error(self, message: str | Exception = "") -> None: """ Echoes an error message to the user """ - self.echo(message, type="ERROR") + self.echo(str(message), type="ERROR") diff --git a/robusta_krr/utils/service_discovery.py b/robusta_krr/utils/service_discovery.py index 42890d3..20138ed 100644 --- a/robusta_krr/utils/service_discovery.py +++ b/robusta_krr/utils/service_discovery.py @@ -1,4 +1,5 @@ from typing import Optional +from abc import ABC, abstractmethod from cachetools import TTLCache from kubernetes import client @@ -82,3 +83,9 @@ class ServiceDiscovery(Configurable): return ingress_url return None + + +class MetricsServiceDiscovery(ServiceDiscovery, ABC): + @abstractmethod + def find_metrics_url(self, *, api_client: Optional[ApiClient] = None) -> Optional[str]: + pass -- cgit v1.2.3