From 306e52eae3333e3d70a669e058dc8efdfef6460d Mon Sep 17 00:00:00 2001 From: Mikhail Sveshnikov Date: Thu, 11 Jul 2024 23:37:22 +0400 Subject: [PATCH] flaky windows tests (#1198) --- setup.cfg | 1 + setup.py | 1 + src/evidently/collector/app.py | 4 +- src/evidently/pydantic_utils.py | 4 +- src/evidently/report/report.py | 5 +- src/evidently/tracing/__init__.py | 6 ++- src/evidently/ui/api/projects.py | 45 +++++++++++++++- src/evidently/ui/components/security.py | 6 +-- src/evidently/ui/local_service.py | 53 +++++++++++++++++++ src/evidently/ui/storage/local/watcher.py | 6 ++- src/evidently/ui/workspace/cloud.py | 41 +++++++++++++-- src/evidently/ui/workspace/remote.py | 62 +++++++++++++++++++---- tests/collector/conftest.py | 2 +- tests/collector/test_app.py | 2 + tests/multitest/metrics/test_all.py | 7 ++- tests/ui/conftest.py | 7 ++- tests/ui/test_app.py | 2 + 17 files changed, 223 insertions(+), 31 deletions(-) diff --git a/setup.cfg b/setup.cfg index 74b2a35a54..00c9054caa 100644 --- a/setup.cfg +++ b/setup.cfg @@ -105,3 +105,4 @@ testpaths=tests python_classes=*Test markers: slow: slow tests + asyncio: async tests diff --git a/setup.py b/setup.py index de993a7d5c..0d85fb7069 100644 --- a/setup.py +++ b/setup.py @@ -97,6 +97,7 @@ "httpx==0.24.1", "ruff==0.3.7", "pre-commit==3.5.0", + "pytest-asyncio==0.23.7", ], "llm": [ "openai>=1.16.2", diff --git a/src/evidently/collector/app.py b/src/evidently/collector/app.py index df04dad308..dfc7db53a4 100644 --- a/src/evidently/collector/app.py +++ b/src/evidently/collector/app.py @@ -237,8 +237,8 @@ def reraise(_, exception: Exception): "service": Provide(lambda: service, use_cache=True, sync_to_thread=False), "storage": Provide(lambda: service.storage, use_cache=True, sync_to_thread=False), "parsed_json": Provide(parse_json, sync_to_thread=False), - "service_config_path": Provide(lambda: config_path), - "service_workspace": Provide(lambda: os.path.dirname(config_path)), + "service_config_path": Provide(lambda: config_path, sync_to_thread=False), + "service_workspace": Provide(lambda: os.path.dirname(config_path), sync_to_thread=False), }, middleware=[auth_middleware_factory], guards=[is_authenticated], diff --git a/src/evidently/pydantic_utils.py b/src/evidently/pydantic_utils.py index ec5cbf5abf..93d740451b 100644 --- a/src/evidently/pydantic_utils.py +++ b/src/evidently/pydantic_utils.py @@ -119,7 +119,7 @@ def all_subclasses(cls: Type[T]) -> Set[Type[T]]: def register_type_alias(base_class: Type["PolymorphicModel"], classpath: str, alias: str): key = (base_class, alias) - if key in TYPE_ALIASES and TYPE_ALIASES[key] != classpath: + if key in TYPE_ALIASES and TYPE_ALIASES[key] != classpath and "PYTEST_CURRENT_TEST" not in os.environ: warnings.warn(f"Duplicate key {key} in alias map") TYPE_ALIASES[key] = classpath @@ -129,7 +129,7 @@ def register_loaded_alias(base_class: Type["PolymorphicModel"], cls: Type["Polym raise ValueError(f"Cannot register alias: {cls.__name__} is not subclass of {base_class.__name__}") key = (base_class, alias) - if key in LOADED_TYPE_ALIASES and LOADED_TYPE_ALIASES[key] != cls: + if key in LOADED_TYPE_ALIASES and LOADED_TYPE_ALIASES[key] != cls and "PYTEST_CURRENT_TEST" not in os.environ: warnings.warn(f"Duplicate key {key} in alias map") LOADED_TYPE_ALIASES[key] = cls diff --git a/src/evidently/report/report.py b/src/evidently/report/report.py index 471c0220b2..70b687417c 100644 --- a/src/evidently/report/report.py +++ b/src/evidently/report/report.py @@ -111,7 +111,10 @@ def run( column_mapping, self.options.data_definition_options.categorical_features_cardinality, ) - + if METRIC_GENERATORS in self.metadata: + del self.metadata[METRIC_GENERATORS] + if METRIC_PRESETS in self.metadata: + del self.metadata[METRIC_PRESETS] # get each item from metrics/presets and add to metrics list # do it in one loop because we want to save metrics and presets order for item in self.metrics: diff --git a/src/evidently/tracing/__init__.py b/src/evidently/tracing/__init__.py index 2949cecf15..304eef49c3 100644 --- a/src/evidently/tracing/__init__.py +++ b/src/evidently/tracing/__init__.py @@ -13,6 +13,7 @@ from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor from opentelemetry.sdk.trace.export import SpanExporter +from requests import Response from evidently.ui.workspace.cloud import ACCESS_TOKEN_COOKIE from evidently.ui.workspace.cloud import CloudMetadataStorage @@ -107,14 +108,15 @@ def _create_tracer_provider( ) cloud = CloudMetadataStorage(_address, _api_key, ACCESS_TOKEN_COOKIE.key) - datasets = cloud._request("/api/datasets", "GET").json()["datasets"] + datasets_response: Response = cloud._request("/api/datasets", "GET") + datasets = datasets_response.json()["datasets"] _export_id = None for dataset in datasets: if dataset["name"] == _export_name: _export_id = dataset["id"] break if _export_id is None: - resp = cloud._request( + resp: Response = cloud._request( "/api/datasets/tracing", "POST", query_params={"team_id": _team_id}, diff --git a/src/evidently/ui/api/projects.py b/src/evidently/ui/api/projects.py index e52c75a9e4..5af6800855 100644 --- a/src/evidently/ui/api/projects.py +++ b/src/evidently/ui/api/projects.py @@ -30,6 +30,7 @@ from evidently.ui.base import Permission from evidently.ui.base import Project from evidently.ui.base import ProjectManager +from evidently.ui.base import SnapshotMetadata from evidently.ui.dashboards.base import DashboardPanel from evidently.ui.dashboards.reports import DashboardPanelCounter from evidently.ui.dashboards.reports import DashboardPanelDistribution @@ -59,6 +60,21 @@ def list_reports( return reports +@get("/{project_id:uuid}/snapshots", sync_to_thread=True) +def list_snapshots( + project_id: Annotated[uuid.UUID, Parameter(title="id of project")], + project_manager: Annotated[ProjectManager, Dependency(skip_validation=True)], + log_event: Callable, + user_id: UserID, +) -> List[SnapshotMetadata]: + project = project_manager.get_project(user_id, project_id) + if project is None: + raise HTTPException(status_code=404, detail="project not found") + snapshots = project_manager.list_snapshots(user_id, project_id) + log_event("list_snapshots", reports_count=len(snapshots)) + return snapshots + + @get("/", sync_to_thread=True) def list_projects( project_manager: Annotated[ProjectManager, Dependency(skip_validation=True)], @@ -236,6 +252,31 @@ def get_snapshot_data( return json.dumps(asdict(info), cls=NumpyEncoder) +@get("/{project_id:uuid}/{snapshot_id:uuid}/metadata", sync_to_thread=True) +def get_snapshot_metadata( + project_id: Annotated[uuid.UUID, Parameter(title="id of project")], + snapshot_id: Annotated[uuid.UUID, Parameter(title="id of snapshot")], + project_manager: Annotated[ProjectManager, Dependency(skip_validation=True)], + log_event: Callable, + user_id: UserID, +) -> SnapshotMetadata: + project = project_manager.get_project(user_id, project_id) + if project is None: + raise HTTPException(status_code=404, detail="Project not found") + snapshot_meta = project.get_snapshot_metadata(snapshot_id) + if snapshot_meta is None: + raise HTTPException(status_code=404, detail="Snapshot not found") + log_event( + "get_snapshot_metadata", + snapshot_type="report" if snapshot_meta.is_report else "test_suite", + metric_presets=snapshot_meta.metadata.get(METRIC_PRESETS, []), + metric_generators=snapshot_meta.metadata.get(METRIC_GENERATORS, []), + test_presets=snapshot_meta.metadata.get(TEST_PRESETS, []), + test_generators=snapshot_meta.metadata.get(TEST_GENERATORS, []), + ) + return snapshot_meta + + @get("/{project_id:uuid}/dashboard/panels", sync_to_thread=True) def list_project_dashboard_panels( project_id: Annotated[uuid.UUID, Parameter(title="id of project")], @@ -253,7 +294,7 @@ def list_project_dashboard_panels( # We need this endpoint to export # some additional models to open api schema @get("/models/additional") -def additional_models() -> ( +async def additional_models() -> ( List[ Union[ DashboardInfoModel, @@ -362,6 +403,8 @@ def create_projects_api(guard: Callable) -> Router: get_snapshot_download, list_project_dashboard_panels, project_dashboard, + list_snapshots, + get_snapshot_metadata, ], ), Router( diff --git a/src/evidently/ui/components/security.py b/src/evidently/ui/components/security.py index 4fa986266e..0692262a5b 100644 --- a/src/evidently/ui/components/security.py +++ b/src/evidently/ui/components/security.py @@ -79,9 +79,9 @@ class SimpleSecurity(SecurityComponent): def get_dependencies(self, ctx: ComponentContext) -> Dict[str, Provide]: return { "user_id": Provide(get_user_id), - "security": Provide(self.get_security), - "security_config": Provide(lambda: self), - "auth_manager": Provide(lambda: NoopAuthManager()), + "security": Provide(self.get_security, sync_to_thread=False), + "security_config": Provide(lambda: self, sync_to_thread=False), + "auth_manager": Provide(lambda: NoopAuthManager(), sync_to_thread=False), } diff --git a/src/evidently/ui/local_service.py b/src/evidently/ui/local_service.py index 1e024990ad..c48ab119c6 100644 --- a/src/evidently/ui/local_service.py +++ b/src/evidently/ui/local_service.py @@ -1,5 +1,9 @@ +import logging +import time + from litestar import Request from litestar import Response +from litestar.logging import LoggingConfig from evidently.ui.api.projects import create_projects_api from evidently.ui.api.service import service_api @@ -13,6 +17,7 @@ from evidently.ui.components.storage import StorageComponent from evidently.ui.components.telemetry import TelemetryComponent from evidently.ui.config import AppConfig +from evidently.ui.config import ConfigContext from evidently.ui.errors import EvidentlyServiceError @@ -32,8 +37,56 @@ def get_route_handlers(self, ctx: ComponentContext): def apply(self, ctx: ComponentContext, builder: AppBuilder): super().apply(ctx, builder) + assert isinstance(ctx, ConfigContext) builder.exception_handlers[EvidentlyServiceError] = evidently_service_exception_handler builder.kwargs["debug"] = self.debug + if self.debug: + log_config = create_logging() + builder.kwargs["logging_config"] = LoggingConfig(**log_config) + + +def create_logging() -> dict: + logging.Formatter.converter = time.gmtime + return { + "version": 1, + "log_exceptions": "always", + "disable_existing_loggers": False, + "formatters": { + "default": { + "()": "logging.Formatter", + "format": "%(asctime)s %(levelname)-8s %(name)-15s %(message)s", + "datefmt": "%Y-%m-%dT%H:%M:%SZ", + }, + "access": { + "()": "logging.Formatter", + "format": "%(asctime)s %(levelname)-8s %(name)-15s %(message)s", + "datefmt": "%Y-%m-%dT%H:%M:%SZ", + }, + "standard": { + "()": "logging.Formatter", + "format": "%(asctime)s %(levelname)-8s %(name)-15s %(message)s", + "datefmt": "%Y-%m-%dT%H:%M:%SZ", + }, + }, + "handlers": { + "default": { + "formatter": "default", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout", + }, + "access": { + "formatter": "access", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout", + }, + }, + "loggers": { + "litestar": {"handlers": ["default"]}, + "uvicorn": {"handlers": ["default"], "level": "INFO", "propagate": False}, + "uvicorn.error": {"level": "INFO"}, + "uvicorn.access": {"handlers": ["access"], "level": "INFO", "propagate": False}, + }, + } class LocalConfig(AppConfig): diff --git a/src/evidently/ui/storage/local/watcher.py b/src/evidently/ui/storage/local/watcher.py index 04db60f1fa..b220f342d3 100644 --- a/src/evidently/ui/storage/local/watcher.py +++ b/src/evidently/ui/storage/local/watcher.py @@ -119,5 +119,7 @@ def on_snapshot_event(self, event): or event.event_type == EVENT_TYPE_MOVED and not os.path.exists(event.src_path) ): - del self.state.snapshots[pid][sid] - del self.state.snapshot_data[pid][sid] + if pid in self.state.snapshots and sid in self.state.snapshots[pid]: + del self.state.snapshots[pid][sid] + if pid in self.state.snapshot_data and sid in self.state.snapshot_data[pid]: + del self.state.snapshot_data[pid][sid] diff --git a/src/evidently/ui/workspace/cloud.py b/src/evidently/ui/workspace/cloud.py index 94dcbc999b..955a8581b8 100644 --- a/src/evidently/ui/workspace/cloud.py +++ b/src/evidently/ui/workspace/cloud.py @@ -2,13 +2,17 @@ from typing import BinaryIO from typing import Dict from typing import List +from typing import Literal from typing import NamedTuple from typing import Optional +from typing import Type from typing import Union +from typing import overload from uuid import UUID import pandas as pd from requests import HTTPError +from requests import Response from evidently.ui.api.models import OrgModel from evidently.ui.api.models import TeamModel @@ -24,6 +28,7 @@ from evidently.ui.workspace.remote import NoopBlobStorage from evidently.ui.workspace.remote import NoopDataStorage from evidently.ui.workspace.remote import RemoteMetadataStorage +from evidently.ui.workspace.remote import T from evidently.ui.workspace.view import WorkspaceView TOKEN_HEADER_NAME = "X-Evidently-Token" @@ -84,17 +89,45 @@ def _prepare_request( r.cookies[self.token_cookie_name] = self.jwt_token return r + @overload def _request( self, path: str, method: str, query_params: Optional[dict] = None, body: Optional[dict] = None, - response_model=None, + response_model: Type[T] = ..., cookies=None, headers: Dict[str, str] = None, form_data: bool = False, - ): + ) -> T: + pass + + @overload + def _request( + self, + path: str, + method: str, + query_params: Optional[dict] = None, + body: Optional[dict] = None, + response_model: Literal[None] = None, + cookies=None, + headers: Dict[str, str] = None, + form_data: bool = False, + ) -> Response: + pass + + def _request( + self, + path: str, + method: str, + query_params: Optional[dict] = None, + body: Optional[dict] = None, + response_model: Optional[Type[T]] = None, + cookies=None, + headers: Dict[str, str] = None, + form_data: bool = False, + ) -> Union[Response, T]: try: res = super()._request( path=path, @@ -142,7 +175,7 @@ def create_team(self, team: Team, org_id: OrgID = None) -> TeamModel: def add_dataset( self, file: BinaryIO, name: str, org_id: OrgID, team_id: TeamID, description: Optional[str] ) -> DatasetID: - response = self._request( + response: Response = self._request( "/api/datasets/", "POST", body={"name": name, "description": description, "file": file}, @@ -152,7 +185,7 @@ def add_dataset( return DatasetID(response.json()["dataset_id"]) def load_dataset(self, dataset_id: DatasetID) -> pd.DataFrame: - response = self._request(f"/api/datasets/{dataset_id}/download", "GET") + response: Response = self._request(f"/api/datasets/{dataset_id}/download", "GET") return pd.read_parquet(BytesIO(response.content)) diff --git a/src/evidently/ui/workspace/remote.py b/src/evidently/ui/workspace/remote.py index d4f5abb4e1..8a64a4c869 100644 --- a/src/evidently/ui/workspace/remote.py +++ b/src/evidently/ui/workspace/remote.py @@ -7,13 +7,17 @@ from json import JSONDecodeError from typing import Dict from typing import List +from typing import Literal from typing import Optional from typing import Set from typing import Type +from typing import TypeVar from typing import Union +from typing import overload from urllib.error import HTTPError from requests import Request +from requests import Response from requests import Session from evidently._pydantic_compat import parse_obj_as @@ -32,7 +36,7 @@ from evidently.ui.dashboards.base import ReportFilter from evidently.ui.dashboards.test_suites import TestFilter from evidently.ui.errors import EvidentlyServiceError -from evidently.ui.storage.common import NO_USER +from evidently.ui.errors import ProjectNotFound from evidently.ui.storage.common import SECRET_HEADER_NAME from evidently.ui.storage.common import NoopAuthManager from evidently.ui.type_aliases import ZERO_UUID @@ -45,6 +49,8 @@ from evidently.ui.workspace.view import WorkspaceView from evidently.utils import NumpyEncoder +T = TypeVar("T") + class RemoteBase: def get_url(self): @@ -82,17 +88,45 @@ def _prepare_request( cookies=cookies, ) + @overload def _request( self, path: str, method: str, query_params: Optional[dict] = None, body: Optional[dict] = None, - response_model=None, + response_model: Type[T] = ..., cookies=None, headers: Dict[str, str] = None, form_data: bool = False, - ): + ) -> T: + pass + + @overload + def _request( + self, + path: str, + method: str, + query_params: Optional[dict] = None, + body: Optional[dict] = None, + response_model: Literal[None] = None, + cookies=None, + headers: Dict[str, str] = None, + form_data: bool = False, + ) -> Response: + pass + + def _request( + self, + path: str, + method: str, + query_params: Optional[dict] = None, + body: Optional[dict] = None, + response_model: Optional[Type[T]] = None, + cookies=None, + headers: Dict[str, str] = None, + form_data: bool = False, + ) -> Union[Response, T]: request = self._prepare_request(path, method, query_params, body, cookies, headers, form_data=form_data) s = Session() response = s.send(request.prepare()) @@ -173,18 +207,28 @@ def delete_snapshot(self, project_id: ProjectID, snapshot_id: SnapshotID): return self._request(f"/api/projects/{project_id}/{snapshot_id}", "DELETE") def search_project(self, project_name: str, project_ids: Optional[Set[ProjectID]]) -> List[Project]: - return [ - p.bind(self, NO_USER.id) - for p in self._request(f"/api/projects/search/{project_name}", "GET", response_model=List[Project]) - ] + return self._request(f"/api/projects/search/{project_name}", "GET", response_model=List[Project]) def list_snapshots( self, project_id: ProjectID, include_reports: bool = True, include_test_suites: bool = True ) -> List[SnapshotMetadata]: - raise NotImplementedError + project = self.get_project(project_id) + if project is None: + raise ProjectNotFound() + return [ + sm.bind(project) + for sm in self._request( + f"/api/projects/{project_id}/snapshots", "GET", response_model=List[SnapshotMetadata] + ) + ] def get_snapshot_metadata(self, project_id: ProjectID, snapshot_id: SnapshotID) -> SnapshotMetadata: - raise NotImplementedError + project = self.get_project(project_id) + if project is None: + raise ProjectNotFound() + return self._request( + f"/api/projects/{project_id}/{snapshot_id}/metadata", "GET", response_model=SnapshotMetadata + ).bind(project) def update_project(self, project: Project) -> Project: return self._request(f"/api/projects/{project.id}/info", "POST", body=project.dict(), response_model=Project) diff --git a/tests/collector/conftest.py b/tests/collector/conftest.py index f7608aca60..8d42a3e31d 100644 --- a/tests/collector/conftest.py +++ b/tests/collector/conftest.py @@ -32,7 +32,7 @@ def collector_test_client(tmp_path): state = {} @get("/_init_tests") - def test_init(service: CollectorServiceConfig, service_workspace: str) -> None: + async def test_init(service: CollectorServiceConfig, service_workspace: str) -> None: state["config"] = service state["workspace"] = service_workspace diff --git a/tests/collector/test_app.py b/tests/collector/test_app.py index f4ec157f5e..247ed39467 100644 --- a/tests/collector/test_app.py +++ b/tests/collector/test_app.py @@ -1,5 +1,6 @@ import asyncio import os.path +import sys import pandas as pd import pytest @@ -17,6 +18,7 @@ @pytest.mark.asyncio +@pytest.mark.skipif(sys.version_info < (3, 10), reason="loop not available on python < 3.10") async def test_create_collector( collector_test_client: TestClient, collector_service_config: CollectorServiceConfig, mock_collector_config ): diff --git a/tests/multitest/metrics/test_all.py b/tests/multitest/metrics/test_all.py index 7634ef5631..4c300026a8 100644 --- a/tests/multitest/metrics/test_all.py +++ b/tests/multitest/metrics/test_all.py @@ -21,7 +21,10 @@ from tests.multitest.metrics.conftest import metric_fixtures -def _check_dataframe(report: Report): +def _check_dataframe(report: Report, metric: Metric): + if not metric.__class__.result_type().__config__.pd_include: + # skipping not supported + return df = report.as_dataframe() assert isinstance(df, pd.DataFrame) @@ -60,7 +63,7 @@ def test_metric(tmetric: TestMetric, tdataset: TestDataset, outcome: TestOutcome report._inner_suite.raise_for_error() assert report.show() assert report.json() - _check_dataframe(report) + _check_dataframe(report, tmetric.metric) outcome.check(report) diff --git a/tests/ui/conftest.py b/tests/ui/conftest.py index 4c61c9f215..e0a2dfda27 100644 --- a/tests/ui/conftest.py +++ b/tests/ui/conftest.py @@ -16,6 +16,7 @@ from evidently.ui.components.base import ComponentContext from evidently.ui.components.storage import LocalStorageComponent from evidently.ui.local_service import LocalConfig +from evidently.ui.local_service import LocalServiceComponent from evidently.ui.security.service import SecurityService from evidently.utils import NumpyEncoder @@ -32,7 +33,7 @@ class Config: def get_route_handlers(self, ctx: ComponentContext): @get("/tests_setup") - def tests_setup(project_manager: ProjectManager, security: SecurityService) -> None: + async def tests_setup(project_manager: ProjectManager, security: SecurityService) -> None: self.app.state["pm"] = project_manager self.app.state["security"] = security @@ -78,7 +79,9 @@ def _dumps(obj: BaseModel): @pytest.fixture def test_client(tmp_path): config = LocalConfig( - storage=LocalStorageComponent(path=str(tmp_path)), additional_components={"_setup_tests": TestsSetupComponent()} + storage=LocalStorageComponent(path=str(tmp_path)), + service=LocalServiceComponent(debug=True), + additional_components={"_setup_tests": TestsSetupComponent()}, ) return TestClient(create_app(config=config)) diff --git a/tests/ui/test_app.py b/tests/ui/test_app.py index 441736ffda..031689ca39 100644 --- a/tests/ui/test_app.py +++ b/tests/ui/test_app.py @@ -1,6 +1,7 @@ import datetime import json import os +import time import uuid from copy import deepcopy from typing import List @@ -172,6 +173,7 @@ def test_delete_snapshot(test_client: TestClient, project_manager: ProjectManage project = project_manager.add_project(mock_project, ZERO_UUID, ZERO_UUID) project_manager.add_snapshot(ZERO_UUID, project.id, mock_snapshot) assert len(project_manager.list_snapshots(ZERO_UUID, project.id)) == 1 + time.sleep(0.1) # try to avoid WinError 32 error (file used by another process) r = test_client.delete(f"/api/projects/{project.id}/{mock_snapshot.id}") r.raise_for_status()