ref(deletes): move delete logic out of snuba/web

snuba/admin/views.py

@@ -70,6 +70,11 @@
 from snuba.datasets.factory import InvalidDatasetError, get_enabled_dataset_names
 from snuba.datasets.storages.factory import get_storage, get_writable_storage
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.delete_query import (
+    DeletesNotEnabledError,
+    delete_from_storage,
+    deletes_are_enabled,
+)
 from snuba.manual_jobs.runner import (
     list_job_specs,
     list_job_specs_with_status,
@@ -90,11 +95,6 @@
 from snuba.state.explain_meta import explain_cleanup, get_explain_meta
 from snuba.utils.metrics.timer import Timer
 from snuba.utils.registered_class import InvalidConfigKeyError
-from snuba.web.delete_query import (
-    DeletesNotEnabledError,
-    delete_from_storage,
-    deletes_are_enabled,
-)
 from snuba.web.rpc import RPCEndpoint, list_all_endpoint_names, run_rpc_handler
 from snuba.web.rpc.storage_routing.routing_strategies.storage_routing import (
     BaseRoutingStrategy,

snuba/cli/lw_deletions_consumer.py

@@ -19,11 +19,11 @@
 from snuba.datasets.storages.factory import get_writable_storage
 from snuba.datasets.storages.storage_key import StorageKey
 from snuba.environment import setup_logging, setup_sentry
+from snuba.lw_deletions.bulk_delete_query import STORAGE_TOPIC
 from snuba.lw_deletions.formatters import STORAGE_FORMATTER
 from snuba.lw_deletions.strategy import LWDeletionsConsumerStrategyFactory
 from snuba.utils.metrics.wrapper import MetricsWrapper
 from snuba.utils.streams.metrics_adapter import StreamMetricsAdapter
-from snuba.web.bulk_delete_query import STORAGE_TOPIC
 
 # A longer batch time for deletes is reasonable
 # since we want fewer mutations
@@ -126,9 +126,7 @@ def handler(signum: int, frame: Any) -> None:
             "consumer_group": consumer_group,
             "storage": storage,
         }
-        metrics = MetricsWrapper(
-            environment.metrics, "lw_deletions_consumer", tags=metrics_tags
-        )
+        metrics = MetricsWrapper(environment.metrics, "lw_deletions_consumer", tags=metrics_tags)
         configure_metrics(StreamMetricsAdapter(metrics), force=True)
         consumer_config = resolve_consumer_config(
             storage_names=[storage],

snuba/web/bulk_delete_query.py → snuba/lw_deletions/bulk_delete_query.py

@@ -15,6 +15,13 @@
 from snuba.datasets.deletion_settings import DeletionSettings, get_trace_item_type_name
 from snuba.datasets.storage import WritableTableStorage
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.delete_query import (
+    DeletesNotEnabledError,
+    _construct_condition,
+    _enforce_max_rows,
+    _get_attribution_info,
+    deletes_are_enabled,
+)
 from snuba.lw_deletions.types import AttributeConditions, ConditionsBag, ConditionsType
 from snuba.query.conditions import combine_or_conditions
 from snuba.query.data_source.simple import Table
@@ -28,13 +35,6 @@
 from snuba.utils.schemas import ColumnValidator, InvalidColumnType
 from snuba.utils.streams.configuration_builder import build_kafka_producer_configuration
 from snuba.utils.streams.topics import Topic
-from snuba.web.delete_query import (
-    DeletesNotEnabledError,
-    _construct_condition,
-    _enforce_max_rows,
-    _get_attribution_info,
-    deletes_are_enabled,
-)
 
 metrics = MetricsWrapper(environment.metrics, "snuba.delete")
 logger = logging.getLogger(__name__)
@@ -226,7 +226,10 @@ def delete_from_storage(
         _validate_attribute_conditions(attribute_conditions, delete_settings)
 
         if not get_int_config("permit_delete_by_attribute", default=0):
-            metrics.increment("delete_query.delete_ignored")
+            logger.error(
+                "valid attribute_conditions passed to delete_from_storage, but delete will be ignored "
+                "as functionality is not yet launched (permit_delete_by_attribute=0)"
+            )
             return {}
 
     attr_info = _get_attribution_info(attribution_info)

snuba/lw_deletions/formatters.py

@@ -15,8 +15,11 @@
 from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey
 
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.bulk_delete_query import (
+    DeleteQueryMessage,
+    WireAttributeCondition,
+)
 from snuba.lw_deletions.types import AttributeConditions, ConditionsBag
-from snuba.web.bulk_delete_query import DeleteQueryMessage, WireAttributeCondition
 
 
 class Formatter(ABC):

snuba/lw_deletions/strategy.py

@@ -19,20 +19,23 @@
 from snuba.datasets.storage import WritableTableStorage
 from snuba.datasets.storages.storage_key import StorageKey
 from snuba.lw_deletions.batching import BatchStepCustom, ValuesBatch
+from snuba.lw_deletions.bulk_delete_query import (
+    construct_or_conditions,
+    construct_query,
+)
+from snuba.lw_deletions.delete_query import (
+    TooManyOngoingMutationsError,
+    _execute_query,
+    _num_ongoing_mutations,
+)
 from snuba.lw_deletions.formatters import Formatter
 from snuba.lw_deletions.types import ConditionsBag
 from snuba.query.allocation_policies import AllocationPolicyViolations
 from snuba.query.query_settings import HTTPQuerySettings
 from snuba.state import get_int_config, get_str_config
 from snuba.utils.metrics import MetricsBackend
 from snuba.web import QueryException
-from snuba.web.bulk_delete_query import construct_or_conditions, construct_query
 from snuba.web.constants import LW_DELETE_NON_RETRYABLE_CLICKHOUSE_ERROR_CODES
-from snuba.web.delete_query import (
-    TooManyOngoingMutationsError,
-    _execute_query,
-    _num_ongoing_mutations,
-)
 
 TPayload = TypeVar("TPayload")
 

snuba/web/rpc/v1/endpoint_delete_trace_items.py

@@ -11,8 +11,8 @@
 from snuba.attribution.appid import AppID
 from snuba.datasets.storages.factory import get_writable_storage
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.bulk_delete_query import delete_from_storage
 from snuba.lw_deletions.types import AttributeConditions
-from snuba.web.bulk_delete_query import delete_from_storage
 from snuba.web.rpc import RPCEndpoint
 from snuba.web.rpc.common.exceptions import BadSnubaRPCRequestException
 

snuba/web/views.py

@@ -52,6 +52,13 @@
 from snuba.datasets.factory import InvalidDatasetError, get_dataset_name
 from snuba.datasets.schemas.tables import TableSchema
 from snuba.datasets.storage import StorageNotAvailable, WritableTableStorage
+from snuba.lw_deletions.bulk_delete_query import (
+    delete_from_storage as bulk_delete_from_storage,
+)
+from snuba.lw_deletions.delete_query import (
+    DeletesNotEnabledError,
+    TooManyOngoingMutationsError,
+)
 from snuba.query.allocation_policies import AllocationPolicyViolations
 from snuba.query.exceptions import InvalidQueryException, QueryPlanException
 from snuba.query.query_settings import HTTPQuerySettings
@@ -72,10 +79,8 @@
 from snuba.utils.metrics.timer import Timer
 from snuba.utils.metrics.util import with_span
 from snuba.web import QueryException, QueryTooLongException
-from snuba.web.bulk_delete_query import delete_from_storage as bulk_delete_from_storage
 from snuba.web.constants import get_http_status_for_clickhouse_error
 from snuba.web.converters import DatasetConverter, EntityConverter, StorageConverter
-from snuba.web.delete_query import DeletesNotEnabledError, TooManyOngoingMutationsError
 from snuba.web.query import parse_and_run_query
 from snuba.web.rpc import run_rpc_handler
 from snuba.writer import BatchWriterEncoderWrapper, WriterTableRow

tests/web/test_bulk_delete_query.py → tests/lw_deletions/test_bulk_delete_query.py

@@ -13,14 +13,14 @@
 from snuba import settings
 from snuba.datasets.storages.factory import get_writable_storage
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.bulk_delete_query import delete_from_storage
+from snuba.lw_deletions.delete_query import DeletesNotEnabledError
 from snuba.lw_deletions.types import AttributeConditions
 from snuba.query.exceptions import InvalidQueryException
 from snuba.state import set_config
 from snuba.utils.manage_topics import create_topics
 from snuba.utils.streams.configuration_builder import get_default_kafka_configuration
 from snuba.utils.streams.topics import Topic
-from snuba.web.bulk_delete_query import delete_from_storage
-from snuba.web.delete_query import DeletesNotEnabledError
 
 # TraceItemType values from sentry_protos
 TRACE_ITEM_TYPE_SPAN = 1
@@ -48,7 +48,7 @@ def get_attribution_info(tenant_ids: Optional[Mapping[str, int | str]] = None) -
     }
 
 
-@patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10)
+@patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10)
 def test_delete_success(mock_enforce_max_row: Mock) -> None:
     admin_client = AdminClient(get_default_kafka_configuration())
     create_topics(admin_client, [Topic.LW_DELETIONS_GENERIC_EVENTS])
@@ -107,8 +107,8 @@ def test_deletes_not_enabled_runtime_config() -> None:
 
 
 @pytest.mark.redis_db
-@patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10)
-@patch("snuba.web.bulk_delete_query.produce_delete_query")
+@patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10)
+@patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query")
 def test_deletes_killswitch(mock_produce_query: Mock, mock_enforce_rows: Mock) -> None:
     storage = get_writable_storage(StorageKey("search_issues"))
     conditions = {"project_id": [1], "group_id": [1, 2, 3, 4]}
@@ -177,8 +177,8 @@ def test_attribute_conditions_valid_occurrence() -> None:
     attr_info = get_attribution_info()
 
     # Mock out _enforce_max_rows to avoid needing actual data
-    with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10):
-        with patch("snuba.web.bulk_delete_query.produce_delete_query") as mock_produce:
+    with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10):
+        with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") as mock_produce:
             # Should not raise an exception, but should return empty dict since
             # functionality is not yet launched (permit_delete_by_attribute=0 by default)
             result = delete_from_storage(storage, conditions, attr_info, attribute_conditions)
@@ -225,8 +225,8 @@ def test_attribute_conditions_missing_item_type() -> None:
 
     # Since item_type is now in AttributeConditions, we need to test a different scenario
     # The validation now should pass, but we need to ensure item_type is also in conditions
-    with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10):
-        with patch("snuba.web.bulk_delete_query.produce_delete_query"):
+    with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10):
+        with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query"):
             # This should now succeed since we're no longer checking conditions dict
             delete_from_storage(storage, conditions, attr_info, attribute_conditions)
 
@@ -268,8 +268,8 @@ def test_attribute_conditions_feature_flag_enabled() -> None:
 
     try:
         # Mock out _enforce_max_rows to avoid needing actual data
-        with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10):
-            with patch("snuba.web.bulk_delete_query.produce_delete_query") as mock_produce:
+        with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10):
+            with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") as mock_produce:
                 # Should process normally and produce a message
                 result = delete_from_storage(storage, conditions, attr_info, attribute_conditions)
 

tests/web/test_delete_query.py → tests/lw_deletions/test_delete_query.py

@@ -11,6 +11,7 @@
 from snuba.configs.configuration import Configuration, ResourceIdentifier
 from snuba.datasets.storages.factory import get_writable_storage
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.delete_query import _execute_query
 from snuba.query.allocation_policies import (
     MAX_THRESHOLD,
     NO_SUGGESTION,
@@ -24,7 +25,6 @@
 from snuba.query.dsl import and_cond, column, equals, literal
 from snuba.query.query_settings import HTTPQuerySettings
 from snuba.web import QueryException
-from snuba.web.delete_query import _execute_query
 
 
 @pytest.mark.clickhouse_db
@@ -113,7 +113,7 @@ def _update_quota_balance(
             return
 
     with mock.patch(
-        "snuba.web.delete_query._get_delete_allocation_policies",
+        "snuba.lw_deletions.delete_query._get_delete_allocation_policies",
         return_value=[
             RejectPolicy(ResourceIdentifier(StorageKey("doesntmatter")), ["a", "b", "c"], {})
         ],

tests/lw_deletions/test_formatters.py

@@ -3,13 +3,16 @@
 import pytest
 from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey
 
+from snuba.lw_deletions.bulk_delete_query import (
+    DeleteQueryMessage,
+    WireAttributeCondition,
+)
 from snuba.lw_deletions.formatters import (
     EAPItemsFormatter,
     Formatter,
     SearchIssuesFormatter,
 )
 from snuba.lw_deletions.types import ConditionsBag, ConditionsType
-from snuba.web.bulk_delete_query import DeleteQueryMessage, WireAttributeCondition
 
 
 def create_delete_query_message(

tests/lw_deletions/test_lw_deletions.py

@@ -13,11 +13,11 @@
 from snuba.datasets.storages.factory import get_writable_storage
 from snuba.datasets.storages.storage_key import StorageKey
 from snuba.lw_deletions.batching import BatchStepCustom
+from snuba.lw_deletions.bulk_delete_query import DeleteQueryMessage
 from snuba.lw_deletions.formatters import SearchIssuesFormatter
 from snuba.lw_deletions.strategy import FormatQuery, increment_by
 from snuba.lw_deletions.types import ConditionsType
 from snuba.utils.streams.topics import Topic as SnubaTopic
-from snuba.web.bulk_delete_query import DeleteQueryMessage
 
 ROWS_CONDITIONS = {
     5: {"project_id": [1], "group_id": [1, 2, 3, 4]},

tests/web/test_max_rows_enforcer.py → tests/lw_deletions/test_max_rows_enforcer.py

@@ -12,11 +12,11 @@
 from snuba.datasets.entities.entity_key import EntityKey
 from snuba.datasets.entities.factory import get_entity
 from snuba.datasets.storages.storage_key import StorageKey
+from snuba.lw_deletions.delete_query import _enforce_max_rows
 from snuba.query.data_source.simple import Table
 from snuba.query.dsl import and_cond, column, equals, literal
 from snuba.query.exceptions import TooManyDeleteRowsException
 from snuba.state import set_config
-from snuba.web.delete_query import _enforce_max_rows
 from tests.base import BaseApiTest
 from tests.datasets.configuration.utils import ConfigurationTest
 from tests.helpers import write_unprocessed_events

tests/test_search_issues_api.py

@@ -104,7 +104,7 @@ def delete_query(
             headers={"referer": "test"},
         )
 
-    @patch("snuba.web.bulk_delete_query.produce_delete_query")
+    @patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query")
     def test_simple_delete(self, mock_produce_delete: Mock) -> None:
         set_config("read_through_cache.short_circuit", 1)
         now = datetime.now().replace(minute=0, second=0, microsecond=0)

tests/web/rpc/v1/test_endpoint_delete_trace_items.py

@@ -119,7 +119,7 @@ def test_valid_trace_id_returns_success_response(self, setup_teardown: Any) -> N
 
         assert MessageToDict(response) == MessageToDict(expected_response)
 
-    @patch("snuba.web.bulk_delete_query.produce_delete_query")
+    @patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query")
     def test_valid_trace_id_produces_bulk_delete_message(
         self, produce_delete_query_mock: Mock, setup_teardown: Any
     ) -> None:
@@ -183,8 +183,8 @@ def test_filters_with_equals_operation_accepted(self) -> None:
             ],
         )
 
-        with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10):
-            with patch("snuba.web.bulk_delete_query.produce_delete_query") as mock_produce:
+        with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10):
+            with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query") as mock_produce:
                 EndpointDeleteTraceItems().execute(message)
 
                 # Verify produce_delete_query was called with attribute_conditions
@@ -221,8 +221,8 @@ def test_filters_with_in_operation_accepted(self) -> None:
             ],
         )
 
-        with patch("snuba.web.bulk_delete_query._enforce_max_rows", return_value=10):
-            with patch("snuba.web.bulk_delete_query.produce_delete_query"):
+        with patch("snuba.lw_deletions.bulk_delete_query._enforce_max_rows", return_value=10):
+            with patch("snuba.lw_deletions.bulk_delete_query.produce_delete_query"):
                 assert isinstance(
                     EndpointDeleteTraceItems().execute(message), DeleteTraceItemsResponse
                 )