Browse Source

Revert "typing(issue_search): Add typing to issue search backends and executors (#29902)"

This reverts commit 38307fc743e5540528719f46d3e2cef68ff6b6d3.

Co-authored-by: dfuller via Slack <dfuller@sentry.io>
Sentry Bot 3 years ago
parent
commit
9f9870bb33
4 changed files with 128 additions and 204 deletions
  1. 0 3
      mypy.ini
  2. 13 25
      src/sentry/search/base.py
  3. 41 82
      src/sentry/search/snuba/backend.py
  4. 74 94
      src/sentry/search/snuba/executors.py

+ 0 - 3
mypy.ini

@@ -51,9 +51,6 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/notifications/**/*.py,
         src/sentry/processing/realtime_metrics/,
         src/sentry/release_health/**/*.py,
-        src/sentry/search/base.py,
-        src/sentry/search/events/constants.py,
-        src/sentry/search/snuba/*.py,
         src/sentry/sentry_metrics/**/*.py,
         src/sentry/shared_integrations/constants.py,
         src/sentry/snuba/outcomes.py,

+ 13 - 25
src/sentry/search/base.py

@@ -1,38 +1,26 @@
-from __future__ import annotations
-
-from datetime import datetime
-from typing import TYPE_CHECKING, Any, FrozenSet, Mapping, Optional, Sequence
-
 from sentry.utils.services import Service
 
 ANY = object()
 
-if TYPE_CHECKING:
-    from sentry.api.event_search import SearchFilter
-    from sentry.models import Environment, Project
-    from sentry.utils.cursors import Cursor, CursorResult
-
 
-class SearchBackend(Service):  # type: ignore
+class SearchBackend(Service):
     __read_methods__ = frozenset(["query"])
-    __write_methods__: FrozenSet[str] = frozenset()
+    __write_methods__ = frozenset()
     __all__ = __read_methods__ | __write_methods__
 
-    def __init__(self, **options: Optional[Mapping[str, Any]]):
+    def __init__(self, **options):
         pass
 
     def query(
         self,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]] = None,
-        sort_by: str = "date",
-        limit: int = 100,
-        cursor: Optional[Cursor] = None,
-        count_hits: bool = False,
-        paginator_options: Optional[Mapping[str, Any]] = None,
-        search_filters: Optional[Sequence[SearchFilter]] = None,
-        date_from: Optional[datetime] = None,
-        date_to: Optional[datetime] = None,
-        max_hits: Optional[int] = None,
-    ) -> CursorResult:
+        projects,
+        tags=None,
+        environments=None,
+        sort_by="date",
+        limit=100,
+        cursor=None,
+        count_hits=False,
+        paginator_options=None,
+        **parameters,
+    ):
         raise NotImplementedError

+ 41 - 82
src/sentry/search/snuba/backend.py

@@ -1,20 +1,16 @@
-from __future__ import annotations
-
 import functools
 from abc import ABCMeta, abstractmethod
 from collections import defaultdict
-from datetime import datetime, timedelta
-from typing import Any, Callable, Dict, Mapping, Optional, Sequence
+from datetime import timedelta
+from typing import Sequence
 
-from django.db.models import Q, QuerySet
+from django.db.models import Q
 from django.utils import timezone
 from django.utils.functional import SimpleLazyObject
 
 from sentry import quotas
-from sentry.api.event_search import SearchFilter
 from sentry.exceptions import InvalidSearchQuery
 from sentry.models import (
-    Environment,
     Group,
     GroupAssignee,
     GroupEnvironment,
@@ -34,17 +30,10 @@ from sentry.models import (
 )
 from sentry.search.base import SearchBackend
 from sentry.search.events.constants import EQUALITY_OPERATORS, OPERATOR_TO_DJANGO
-from sentry.search.snuba.executors import (
-    AbstractQueryExecutor,
-    CdcPostgresSnubaQueryExecutor,
-    PostgresSnubaQueryExecutor,
-)
-from sentry.utils.cursors import Cursor, CursorResult
+from sentry.search.snuba.executors import CdcPostgresSnubaQueryExecutor, PostgresSnubaQueryExecutor
 
 
-def assigned_to_filter(
-    actors: Sequence[User | Team | None], projects: Sequence[Project], field_filter: str = "id"
-) -> Q:
+def assigned_to_filter(actors, projects, field_filter="id"):
     from sentry.models import OrganizationMember, OrganizationMemberTeam, Team
 
     include_none = False
@@ -99,7 +88,7 @@ def assigned_to_filter(
     return query
 
 
-def unassigned_filter(unassigned: bool, projects: Sequence[Project], field_filter: str = "id") -> Q:
+def unassigned_filter(unassigned, projects, field_filter="id"):
     query = Q(
         **{
             f"{field_filter}__in": GroupAssignee.objects.filter(
@@ -112,7 +101,7 @@ def unassigned_filter(unassigned: bool, projects: Sequence[Project], field_filte
     return query
 
 
-def linked_filter(linked: bool, projects: Sequence[Project]) -> Q:
+def linked_filter(linked, projects):
     """
     Builds a filter for whether or not a Group has an issue linked via either
     a PlatformExternalIssue or an ExternalIssue.
@@ -149,9 +138,7 @@ def linked_filter(linked: bool, projects: Sequence[Project]) -> Q:
     return query
 
 
-def first_release_all_environments_filter(
-    versions: Sequence[str], projects: Sequence[Project]
-) -> Q:
+def first_release_all_environments_filter(versions, projects):
     releases = {
         id_: version
         for id_, version in Release.objects.filter(
@@ -177,7 +164,7 @@ def first_release_all_environments_filter(
     )
 
 
-def inbox_filter(inbox: bool, projects: Sequence[Project]) -> Q:
+def inbox_filter(inbox, projects):
     query = Q(groupinbox__id__isnull=False)
     if not inbox:
         query = ~query
@@ -186,9 +173,7 @@ def inbox_filter(inbox: bool, projects: Sequence[Project]) -> Q:
     return query
 
 
-def assigned_or_suggested_filter(
-    owners: Sequence[User | Team | None], projects: Sequence[Project], field_filter: str = "id"
-) -> Q:
+def assigned_or_suggested_filter(owners, projects, field_filter="id"):
     organization_id = projects[0].organization_id
     project_ids = [p.id for p in projects]
 
@@ -284,15 +269,15 @@ class Condition:
     ``QuerySetBuilder``.
     """
 
-    def apply(self, queryset: QuerySet, search_filter: SearchFilter) -> QuerySet:
+    def apply(self, queryset, name, parameters):
         raise NotImplementedError
 
 
 class QCallbackCondition(Condition):
-    def __init__(self, callback: Callable[[Any], QuerySet]):
+    def __init__(self, callback):
         self.callback = callback
 
-    def apply(self, queryset: QuerySet, search_filter: SearchFilter) -> QuerySet:
+    def apply(self, queryset, search_filter):
         value = search_filter.value.raw_value
         q = self.callback(value)
         if search_filter.operator not in ("=", "!=", "IN", "NOT IN"):
@@ -312,17 +297,17 @@ class ScalarCondition(Condition):
     instances
     """
 
-    def __init__(self, field: str, extra: Optional[dict[str, Sequence[int]]] = None):
+    def __init__(self, field, extra=None):
         self.field = field
         self.extra = extra
 
-    def _get_operator(self, search_filter: SearchFilter) -> str:
+    def _get_operator(self, search_filter):
         django_operator = OPERATOR_TO_DJANGO.get(search_filter.operator, "")
         if django_operator:
             django_operator = f"__{django_operator}"
         return django_operator
 
-    def apply(self, queryset: QuerySet, search_filter: SearchFilter) -> QuerySet:
+    def apply(self, queryset, search_filter):
         django_operator = self._get_operator(search_filter)
         qs_method = queryset.exclude if search_filter.operator == "!=" else queryset.filter
 
@@ -334,10 +319,10 @@ class ScalarCondition(Condition):
 
 
 class QuerySetBuilder:
-    def __init__(self, conditions: Mapping[str, Condition]):
+    def __init__(self, conditions):
         self.conditions = conditions
 
-    def build(self, queryset: QuerySet, search_filters: Sequence[SearchFilter]) -> QuerySet:
+    def build(self, queryset, search_filters):
         for search_filter in search_filters:
             name = search_filter.key.name
             if name in self.conditions:
@@ -349,18 +334,18 @@ class QuerySetBuilder:
 class SnubaSearchBackendBase(SearchBackend, metaclass=ABCMeta):
     def query(
         self,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]] = None,
-        sort_by: str = "date",
-        limit: int = 100,
-        cursor: Optional[Cursor] = None,
-        count_hits: bool = False,
-        paginator_options: Optional[Mapping[str, Any]] = None,
-        search_filters: Optional[Sequence[SearchFilter]] = None,
-        date_from: Optional[datetime] = None,
-        date_to: Optional[datetime] = None,
-        max_hits: Optional[int] = None,
-    ) -> CursorResult:
+        projects,
+        environments=None,
+        sort_by="date",
+        limit=100,
+        cursor=None,
+        count_hits=False,
+        paginator_options=None,
+        search_filters=None,
+        date_from=None,
+        date_to=None,
+        max_hits=None,
+    ):
         search_filters = search_filters if search_filters is not None else []
 
         # ensure projects are from same org
@@ -416,14 +401,8 @@ class SnubaSearchBackendBase(SearchBackend, metaclass=ABCMeta):
         )
 
     def _build_group_queryset(
-        self,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]],
-        search_filters: Sequence[SearchFilter],
-        retention_window_start: Optional[datetime],
-        *args: Any,
-        **kwargs: Any,
-    ) -> QuerySet:
+        self, projects, environments, search_filters, retention_window_start, *args, **kwargs
+    ):
         """This method should return a QuerySet of the Group model.
         How you implement it is up to you, but we generally take in the various search parameters
         and filter Group's down using the field's we want to query on in Postgres."""
@@ -440,12 +419,8 @@ class SnubaSearchBackendBase(SearchBackend, metaclass=ABCMeta):
         return group_queryset
 
     def _initialize_group_queryset(
-        self,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]],
-        retention_window_start: Optional[datetime],
-        search_filters: Sequence[SearchFilter],
-    ) -> QuerySet:
+        self, projects, environments, retention_window_start, search_filters
+    ):
         group_queryset = Group.objects.filter(project__in=projects).exclude(
             status__in=[
                 GroupStatus.PENDING_DELETION,
@@ -467,41 +442,25 @@ class SnubaSearchBackendBase(SearchBackend, metaclass=ABCMeta):
         return group_queryset
 
     @abstractmethod
-    def _get_queryset_conditions(
-        self,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]],
-        search_filters: Sequence[SearchFilter],
-    ) -> Mapping[str, Condition]:
+    def _get_queryset_conditions(self, projects, environments, search_filters):
         """This method should return a dict of query set fields and a "Condition" to apply on that field."""
         raise NotImplementedError
 
     @abstractmethod
     def _get_query_executor(
-        self,
-        group_queryset: QuerySet,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]],
-        search_filters: Sequence[SearchFilter],
-        date_from: Optional[datetime],
-        date_to: Optional[datetime],
-    ) -> AbstractQueryExecutor:
+        self, group_queryset, projects, environments, search_filters, date_from, date_to
+    ):
         """This method should return an implementation of the AbstractQueryExecutor
         We will end up calling .query() on the class returned by this method"""
         raise NotImplementedError
 
 
 class EventsDatasetSnubaSearchBackend(SnubaSearchBackendBase):
-    def _get_query_executor(self, *args: Any, **kwargs: Any) -> AbstractQueryExecutor:
+    def _get_query_executor(self, *args, **kwargs):
         return PostgresSnubaQueryExecutor()
 
-    def _get_queryset_conditions(
-        self,
-        projects: Sequence[Project],
-        environments: Optional[Sequence[Environment]],
-        search_filters: Sequence[SearchFilter],
-    ) -> Mapping[str, Condition]:
-        queryset_conditions: Dict[str, Condition] = {
+    def _get_queryset_conditions(self, projects, environments, search_filters):
+        queryset_conditions = {
             "status": QCallbackCondition(lambda statuses: Q(status__in=statuses)),
             "bookmarked_by": QCallbackCondition(
                 lambda users: Q(bookmark_set__project__in=projects, bookmark_set__user__in=users)
@@ -566,5 +525,5 @@ class EventsDatasetSnubaSearchBackend(SnubaSearchBackendBase):
 
 
 class CdcEventsDatasetSnubaSearchBackend(EventsDatasetSnubaSearchBackend):
-    def _get_query_executor(self, *args: Any, **kwargs: Any) -> CdcPostgresSnubaQueryExecutor:
+    def _get_query_executor(self, *args, **kwargs):
         return CdcPostgresSnubaQueryExecutor()

+ 74 - 94
src/sentry/search/snuba/executors.py

@@ -1,18 +1,15 @@
-from __future__ import annotations
-
 import logging
 import time
-from abc import ABCMeta, abstractmethod
+from abc import ABCMeta, abstractmethod, abstractproperty
 from dataclasses import replace
 from datetime import datetime, timedelta
 from hashlib import md5
-from typing import Any, List, Mapping, Sequence, Set, Tuple
+from typing import Any, Mapping, Sequence
 
 import sentry_sdk
 from django.db.models import QuerySet
 from django.utils import timezone
 from snuba_sdk import Direction, Op
-from snuba_sdk.expressions import Expression
 from snuba_sdk.query import Column, Condition, Entity, Function, Join, Limit, OrderBy, Query
 from snuba_sdk.relationships import Relationship
 
@@ -28,7 +25,7 @@ from sentry.utils import json, metrics, snuba
 from sentry.utils.cursors import Cursor, CursorResult
 
 
-def get_search_filter(search_filters: Sequence[SearchFilter], name: str, operator: str) -> Any:
+def get_search_filter(search_filters, name, operator):
     """
     Finds the value of a search filter with the passed name and operator. If
     multiple values are found, returns the most restrictive value
@@ -60,78 +57,65 @@ class AbstractQueryExecutor(metaclass=ABCMeta):
 
     TABLE_ALIAS = ""
 
-    @property
-    @abstractmethod
-    def aggregation_defs(self) -> Sequence[str] | Expression:
+    @abstractproperty
+    def aggregation_defs(self):
         """This method should return a dict of key:value
         where key is a field name for your aggregation
         and value is the aggregation function"""
         raise NotImplementedError
 
-    @property
-    @abstractmethod
-    def dependency_aggregations(self) -> Mapping[str, List[str]]:
+    @abstractproperty
+    def dependency_aggregations(self):
         """This method should return a dict of key:value
         where key is an aggregation_def field name
         and value is a list of aggregation field names that the 'key' aggregation requires."""
         raise NotImplementedError
 
     @property
-    def empty_result(self) -> CursorResult:
+    def empty_result(self):
         return Paginator(Group.objects.none()).get_result()
 
     @property
     @abstractmethod
-    def dataset(self) -> snuba.Dataset:
+    def dataset(self):
         """ "This function should return an enum from snuba.Dataset (like snuba.Dataset.Events)"""
         raise NotImplementedError
 
-    @property
-    @abstractmethod
-    def sort_strategies(self) -> Mapping[str, str]:
-        raise NotImplementedError
-
-    @property
-    @abstractmethod
-    def postgres_only_fields(self) -> Set[str]:
-        raise NotImplementedError
-
     @abstractmethod
     def query(
         self,
-        projects: Sequence[Project],
-        retention_window_start: Optional[datetime],
-        group_queryset: QuerySet,
-        environments: Optional[Sequence[Environment]],
-        sort_by: str,
-        limit: int,
-        cursor: Cursor,
-        count_hits: bool,
-        paginator_options: Optional[Mapping[str, Any]],
-        search_filters: Optional[Sequence[SearchFilter]],
-        date_from: Optional[datetime],
-        date_to: Optional[datetime],
-        max_hits: Optional[int] = None,
-    ) -> CursorResult:
+        projects,
+        retention_window_start,
+        group_queryset,
+        environments,
+        sort_by,
+        limit,
+        cursor,
+        count_hits,
+        paginator_options,
+        search_filters,
+        date_from,
+        date_to,
+    ):
         """This function runs your actual query and returns the results
         We usually return a paginator object, which contains the results and the number of hits"""
         raise NotImplementedError
 
     def snuba_search(
         self,
-        start: datetime,
-        end: datetime,
-        project_ids: Sequence[int],
-        environment_ids: Sequence[int],
-        sort_field: str,
-        organization_id: int,
-        cursor: Optional[Cursor] = None,
-        group_ids: Optional[Sequence[int]] = None,
-        limit: Optional[int] = None,
-        offset: int = 0,
-        get_sample: bool = False,
-        search_filters: Optional[Sequence[SearchFilter]] = None,
-    ) -> Tuple[List[Tuple[int, Any]], int]:
+        start,
+        end,
+        project_ids,
+        environment_ids,
+        sort_field,
+        organization_id,
+        cursor=None,
+        group_ids=None,
+        limit=None,
+        offset=0,
+        get_sample=False,
+        search_filters=None,
+    ):
         """
         Returns a tuple of:
         * a sorted list of (group_id, group_score) tuples sorted descending by score,
@@ -243,12 +227,8 @@ class AbstractQueryExecutor(metaclass=ABCMeta):
         return [(row["group_id"], row[sort_field]) for row in rows], total
 
     def _transform_converted_filter(
-        self,
-        search_filter: Sequence[SearchFilter],
-        converted_filter: Optional[Sequence[any]],
-        project_ids: Sequence[int],
-        environment_ids: Optional[Sequence[int]] = None,
-    ) -> Optional[Sequence[any]]:
+        self, search_filter, converted_filter, project_ids, environment_ids=None
+    ):
         """
         This method serves as a hook - after we convert the search_filter into a
         snuba compatible filter (which converts it in a general dataset
@@ -259,13 +239,13 @@ class AbstractQueryExecutor(metaclass=ABCMeta):
         """
         return converted_filter
 
-    def has_sort_strategy(self, sort_by: str) -> bool:
+    def has_sort_strategy(self, sort_by):
         return sort_by in self.sort_strategies.keys()
 
 
-def trend_aggregation(start: datetime, end: datetime) -> Sequence[str]:
-    middle_date = start + timedelta(seconds=(end - start).total_seconds() * 0.5)
-    middle = datetime.strftime(middle_date, DateArg.date_format)
+def trend_aggregation(start, end):
+    middle = start + timedelta(seconds=(end - start).total_seconds() * 0.5)
+    middle = datetime.strftime(middle, DateArg.date_format)
 
     agg_range_1 = f"countIf(greater(toDateTime('{middle}'), timestamp))"
     agg_range_2 = f"countIf(lessOrEquals(toDateTime('{middle}'), timestamp))"
@@ -318,25 +298,25 @@ class PostgresSnubaQueryExecutor(AbstractQueryExecutor):
     }
 
     @property
-    def dataset(self) -> snuba.Dataset:
+    def dataset(self):
         return snuba.Dataset.Events
 
     def query(
         self,
-        projects: Sequence[Project],
-        retention_window_start: Optional[datetime],
-        group_queryset: QuerySet,
-        environments: Optional[Sequence[Environment]],
-        sort_by: str,
-        limit: int,
-        cursor: Cursor,
-        count_hits: bool,
-        paginator_options: Optional[Mapping[str, Any]],
-        search_filters: Optional[Sequence[SearchFilter]],
-        date_from: Optional[datetime],
-        date_to: Optional[datetime],
-        max_hits: Optional[int] = None,
-    ) -> CursorResult:
+        projects,
+        retention_window_start,
+        group_queryset,
+        environments,
+        sort_by,
+        limit,
+        cursor,
+        count_hits,
+        paginator_options,
+        search_filters,
+        date_from,
+        date_to,
+        max_hits=None,
+    ):
 
         now = timezone.now()
         end = None
@@ -561,22 +541,22 @@ class PostgresSnubaQueryExecutor(AbstractQueryExecutor):
 
     def calculate_hits(
         self,
-        group_ids: Sequence[int],
-        too_many_candidates: bool,
-        sort_field: str,
-        projects: Sequence[Project],
-        retention_window_start: Optional[datetime],
-        group_queryset: Query,
-        environments: Sequence[Environment],
-        sort_by: str,
-        limit: int,
-        cursor: Cursor,
-        count_hits: bool,
-        paginator_options: Mapping[str, Any],
-        search_filters: Sequence[SearchFilter],
-        start: datetime,
-        end: datetime,
-    ) -> Optional[int]:
+        group_ids,
+        too_many_candidates,
+        sort_field,
+        projects,
+        retention_window_start,
+        group_queryset,
+        environments,
+        sort_by,
+        limit,
+        cursor,
+        count_hits,
+        paginator_options,
+        search_filters,
+        start,
+        end,
+    ):
         """
         This method should return an integer representing the number of hits (results) of your search.
         It will return 0 if hits were calculated and there are none.
@@ -738,7 +718,7 @@ class CdcPostgresSnubaQueryExecutor(PostgresSnubaQueryExecutor):
         search_filters: Sequence[SearchFilter],
         date_from: Optional[datetime],
         date_to: Optional[datetime],
-    ) -> Tuple[datetime, datetime, datetime]:
+    ):
         now = timezone.now()
         end = None
         end_params = [_f for _f in [date_to, get_search_filter(search_filters, "date", "<")] if _f]
@@ -768,7 +748,7 @@ class CdcPostgresSnubaQueryExecutor(PostgresSnubaQueryExecutor):
         search_filters: Sequence[SearchFilter],
         date_from: Optional[datetime],
         date_to: Optional[datetime],
-        max_hits: Optional[int] = None,
+        max_hits=None,
     ) -> CursorResult:
 
         if not validate_cdc_search_filters(search_filters):