got rid of print statements, add new next

Author: Rachel Chen

.vscode/settings.json

@@ -66,9 +66,6 @@
   "python.testing.pytestEnabled": true,
   "python.testing.pytestArgs": ["tests"],
   "python.analysis.autoImportCompletions": true,
-  "mypy-type-checker.args": [
-    "--strict"
-  ],
-  "cursorpyright.analysis.autoImportCompletions": true
+  "mypy-type-checker.args": ["--strict"]
 
 }

snuba/admin/notifications/slack/utils.py

@@ -33,7 +33,6 @@ def build_blocks(
 def build_allocation_policy_changed_text(
     data: Any, action: AuditLogAction
 ) -> Optional[str]:
-    # rachelhandlethis
     base = f"*Storage {data['storage']} Allocation Policy Changed:*"
 
     if action == AuditLogAction.ALLOCATION_POLICY_DELETE:

snuba/admin/static/api_client.tsx

@@ -518,12 +518,15 @@ function Client(): Client {
       let body: string;
       let url: string;
 
-      if (configurable_component.type === "allocation_policy") {
-        body = JSON.stringify({ storage: entity.name, policy: configurable_component.name, key, params })
-        url = baseUrl + "allocation_policy_config";
-      } else {
+      if (configurable_component.type === "routing_strategy") {
         body = JSON.stringify({ strategy: configurable_component.name, key, params });
         url = baseUrl + "routing_strategy_config";
+      } else if (entity.type === "strategy" && configurable_component.type === "allocation_policy") {
+        body = JSON.stringify({ strategy: entity.name, policy: configurable_component.name, key, params })
+        url = baseUrl + "allocation_policy_config_for_strategy";
+      } else {
+        body = JSON.stringify({ storage: entity.name, policy: configurable_component.name, key, params })
+        url = baseUrl + "allocation_policy_config_for_storage";
       }
 
       return fetch(url, {

snuba/admin/views.py

@@ -721,7 +721,6 @@ def configs() -> Response:
             assert key != "", "Key cannot be empty string"
 
         except (KeyError, AssertionError) as exc:
-            print("11111")
             return Response(
                 json.dumps({"error": f"Invalid config: {str(exc)}"}),
                 400,
@@ -845,7 +844,6 @@ def config(config_key: str) -> Response:
             state.set_config_description(config_key, new_desc, user=user)
 
         except (KeyError, AssertionError) as exc:
-            print("2222222")
             return Response(
                 json.dumps({"error": f"Invalid config: {str(exc)}"}),
                 400,
@@ -1045,7 +1043,6 @@ def get_routing_strategy_configs(strategy_name: str) -> Response:
         cast(RoutingStrategyConfig, config).to_config_dict()
         for config in configs.values()
     ]
-    print("isithereRACHELITSHERE", serialized_configs)
     return Response(
         json.dumps(serialized_configs), 200, {"Content-Type": "application/json"}
     )
@@ -1055,32 +1052,25 @@ def get_routing_strategy_configs(strategy_name: str) -> Response:
 @check_tool_perms(tools=[AdminTools.CAPACITY_BASED_ROUTING_SYSTEM])
 def set_routing_strategy_config() -> Response:
     data = json.loads(request.data)
-    print("datafsdfsdf", data)
     user = request.headers.get(USER_HEADER_KEY)
 
     try:
-        print("datafsdfsdf", data, data["strategy"])
         strategy, key = (data["strategy"], data["key"])
         params = data.get("params", {})
-        print("params", params)
 
         assert isinstance(strategy, str), "Invalid strategy"
         assert isinstance(key, str), "Invalid key"
-        print("paramsssss", params)
         assert isinstance(params, dict), "Invalid params"
         assert key != "", "Key cannot be empty string"
 
         strategies = list(BaseRoutingStrategy.all_names())
-        print("strategies", strategies)
         strategy = next(
             (s for s in strategies if s == strategy),
             None,
         )
-        print("strategyyyy", strategy)
         assert strategy is not None, "Strategy not found"
 
     except (KeyError, AssertionError) as exc:
-        print("333333")
         return Response(
             json.dumps({"error": f"Invalid config: {str(exc)}"}),
             400,
@@ -1100,7 +1090,6 @@ def set_allocation_policy_config_for_strategy() -> Response:
 
     try:
         strategy, key, policy_name = (data["strategy"], data["key"], data["policy"])
-        print("dkfjlskjflks", data)
 
         params = data.get("params", {})
 

snuba/query/allocation_policies/__init__.py

@@ -325,21 +325,11 @@ class AllocationPolicy(ConfigurableComponent, ABC, metaclass=RegisteredClass):
     * Every allocation policy takes a `storage_key` in its init. The storage_key is like a pseudo-tenant. In different
         environments, storages may be co-located on the same cluster. To facilitate resource sharing, every allocation policy
         knows which storage_key it is serving. This is used to create unique keys for saving the config values.
-        See `__build_runtime_config_key()` for more info.
+        See `_build_runtime_config_key()` for more info.
     * Reiterating that you should no longer use `snuba.state.{get,set}_config()` for runtime configs for a specific Policy. Refer to the Configurations
         section of this docstring for more info.
     """
 
-    # # This component builds redis strings that are delimited by dots, commas, colons
-    # # in order to allow those characters to exist in config we replace them with their
-    # # counterparts on write/read. It may be better to just replace our serialization with JSON
-    # # instead of what we're doing but this is where we're at rn 1/10/24
-    # __KEY_DELIMITERS_TO_ESCAPE_SEQUENCES = {
-    #     ".": "__dot_literal__",
-    #     ",": "__comma_literal__",
-    #     ":": "__colon_literal__",
-    # }
-
     def __init__(
         self,
         storage_key: StorageKey,
@@ -494,7 +484,7 @@ def get_config_value(
             else self.get_configurations()[config_key]
         )
         return get_runtime_config(
-            key=self.__build_runtime_config_key(config_key, params),
+            key=self._build_runtime_config_key(config_key, params),
             default=config_definition.default,
             config_key=CAPMAN_HASH,
         )
@@ -511,7 +501,7 @@ def set_config_value(
         # ensure correct type is stored
         value = config_definition.value_type(value)
         set_runtime_config(
-            key=self.__build_runtime_config_key(config_key, params),
+            key=self._build_runtime_config_key(config_key, params),
             value=value,
             user=user,
             force=True,
@@ -554,22 +544,6 @@ def get_current_configs(self) -> list[dict[str, Any]]:
 
         return detailed_configs
 
-    def __build_runtime_config_key(self, config: str, params: dict[str, Any]) -> str:
-        """
-        Builds a unique key to be used in the actual datastore containing these configs.
-
-        Example return values:
-        - `"mystorage.MyAllocationPolicy.my_config"`            # no params
-        - `"mystorage.MyAllocationPolicy.my_config.a:1,b:2"`    # sorted params
-        """
-        parameters = "."
-        for param in sorted(list(params.keys())):
-            param_sanitized = self.__escape_delimiter_chars(param)
-            value_sanitized = self.__escape_delimiter_chars(params[param])
-            parameters += f"{param_sanitized}:{value_sanitized},"
-        parameters = parameters[:-1]
-        return f"{self.runtime_config_prefix}.{config}{parameters}"
-
     def __deserialize_runtime_config_key(self, key: str) -> tuple[str, dict[str, Any]]:
         """
         Given a raw runtime config key, deconstructs it into it's AllocationPolicy config
@@ -600,25 +574,11 @@ def __deserialize_runtime_config_key(self, key: str) -> tuple[str, dict[str, Any
 
         return config_key, params_dict
 
-    # def __escape_delimiter_chars(self, key: str) -> str:
-    #     if not isinstance(key, str):
-    #         return key
-    #     for (
-    #         delimiter_char,
-    #         escape_sequence,
-    #     ) in self.__KEY_DELIMITERS_TO_ESCAPE_SEQUENCES.items():
-    #         if escape_sequence in str(key):
-    #             raise InvalidConfig(
-    #                 f"{escape_sequence} is not a valid string for a policy config"
-    #             )
-    #         key = key.replace(delimiter_char, escape_sequence)
-    #     return key
-
     def __unescape_delimiter_chars(self, key: str) -> str:
         for (
             delimiter_char,
             escape_sequence,
-        ) in self.__KEY_DELIMITERS_TO_ESCAPE_SEQUENCES.items():
+        ) in self._KEY_DELIMITERS_TO_ESCAPE_SEQUENCES.items():
             key = key.replace(escape_sequence, delimiter_char)
         return key
 
@@ -732,9 +692,6 @@ def storage_key(self) -> StorageKey:
     def component_namespace(self) -> str:
         return "allocation_policy"
 
-    # def get_configurations(self) -> list[Configuration]:
-    #     return cast(list[Configuration], self.config_definitions().values())
-
 
 class PassthroughPolicy(AllocationPolicy):
     def _additional_config_definitions(self) -> list[AllocationPolicyConfig]:

snuba/query/allocation_policies/cross_org.py

@@ -6,13 +6,13 @@
 from snuba.datasets.storages.storage_key import StorageKey
 from snuba.query.allocation_policies import (
     AllocationPolicyConfig,
-    InvalidConfig,
     QueryResultOrError,
     QuotaAllowance,
 )
 from snuba.query.allocation_policies.concurrent_rate_limit import (
     BaseConcurrentRateLimitAllocationPolicy,
 )
+from snuba.query.configuration import InvalidConfig
 from snuba.redis import RedisClientKey, get_redis_client
 from snuba.state.rate_limit import RateLimitParameters
 from snuba.utils.serializable_exception import JsonSerializable

snuba/query/configuration.py

@@ -57,7 +57,7 @@ class ConfigurableComponent:
     # in order to allow those characters to exist in config we replace them with their
     # counterparts on write/read. It may be better to just replace our serialization with JSON
     # instead of what we're doing but this is where we're at rn 1/10/24
-    __KEY_DELIMITERS_TO_ESCAPE_SEQUENCES = {
+    _KEY_DELIMITERS_TO_ESCAPE_SEQUENCES = {
         ".": "__dot_literal__",
         ",": "__comma_literal__",
         ":": "__colon_literal__",
@@ -169,21 +169,21 @@ def _validate_config_params(
 
         return config
 
-    def __escape_delimiter_chars(self, key: str) -> str:
+    def _escape_delimiter_chars(self, key: str) -> str:
         if not isinstance(key, str):
             return key
         for (
             delimiter_char,
             escape_sequence,
-        ) in self.__KEY_DELIMITERS_TO_ESCAPE_SEQUENCES.items():
+        ) in self._KEY_DELIMITERS_TO_ESCAPE_SEQUENCES.items():
             if escape_sequence in str(key):
                 raise InvalidConfig(
                     f"{escape_sequence} is not a valid string for a config"
                 )
             key = key.replace(delimiter_char, escape_sequence)
         return key
 
-    def __build_runtime_config_key(self, config: str, params: dict[str, Any]) -> str:
+    def _build_runtime_config_key(self, config: str, params: dict[str, Any]) -> str:
         """
         Builds a unique key to be used in the actual datastore containing these configs.
 
@@ -193,8 +193,8 @@ def __build_runtime_config_key(self, config: str, params: dict[str, Any]) -> str
         """
         parameters = "."
         for param in sorted(list(params.keys())):
-            param_sanitized = self.__escape_delimiter_chars(param)
-            value_sanitized = self.__escape_delimiter_chars(params[param])
+            param_sanitized = self._escape_delimiter_chars(param)
+            value_sanitized = self._escape_delimiter_chars(params[param])
             parameters += f"{param_sanitized}:{value_sanitized},"
         parameters = parameters[:-1]
         return f"{self.runtime_config_prefix}.{config}{parameters}"
@@ -212,7 +212,7 @@ def delete_config_value(
         """
         self._validate_config_params(config_key, params)
         delete_runtime_config(
-            key=self.__build_runtime_config_key(config_key, params),
+            key=self._build_runtime_config_key(config_key, params),
             user=user,
             config_key=config_key,
         )

snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py

@@ -217,7 +217,6 @@ def _is_highest_accuracy_mode(self, routing_context: RoutingContext) -> bool:
     def get_delete_allocation_policies(self) -> list[AllocationPolicy]:
         return []
 
-    # @classmethod
     def get_allocation_policies(cls) -> list[AllocationPolicy]:
         return [
             ConcurrentRateLimitAllocationPolicy(