fixed params[]

Author: Rachel Chen

snuba/admin/notifications/slack/utils.py

@@ -33,6 +33,7 @@ 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

@@ -35,7 +35,7 @@ import {
   CardinalityQueryResult,
 } from "SnubaAdmin/cardinality_analyzer/types";
 
-import { AllocationPolicy, Configuration, Entity } from "SnubaAdmin/capacity_management/types";
+import { AllocationPolicy, ConfigurableComponent, Configuration, Entity } from "SnubaAdmin/capacity_management/types";
 
 import { ReplayInstruction, Topic } from "SnubaAdmin/dead_letter_queue/types";
 import { AutoReplacementsBypassProjectsData } from "SnubaAdmin/auto_replacements_bypass_projects/types";
@@ -89,14 +89,14 @@ interface Client {
   getRoutingStrategyConfigs: (strategy_name: string) => Promise<Configuration[]>;
   setConfiguration: (
     entity: Entity,
-    configurable_component_name: string,
+    configurable_component: ConfigurableComponent,
     key: string,
     value: string,
     params: object,
   ) => Promise<void>;
   deleteConfiguration: (
     entity: Entity,
-    configurable_component_name: string,
+    configurable_component: ConfigurableComponent,
     key: string,
     params: object,
   ) => Promise<void>;
@@ -475,24 +475,25 @@ function Client(): Client {
     },
     setConfiguration: (
       entity: Entity,
-      configurable_component_name: string,
+      configurable_component: ConfigurableComponent,
       key: string,
       value: string,
       params: object,
     ) => {
-      console.log("dowenotheithere")
       let body: string;
       let url: string;
 
-      console.log("paramskdjlak", params)
-
-      if (entity.type === "storage") {
-        body = JSON.stringify({ storage: entity.name, policy: configurable_component_name, key, value, params })
-        url = baseUrl + "allocation_policy_config";
-      } else {
-        body = JSON.stringify({ strategy: configurable_component_name, key, value, params });
+      if (configurable_component.type === "routing_strategy") {
+        body = JSON.stringify({ strategy: configurable_component.name, key, value, 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, value, params })
+        url = baseUrl + "allocation_policy_config_for_strategy";
+      } else {
+        body = JSON.stringify({ storage: entity.name, policy: configurable_component.name, key, value, params })
+        url = baseUrl + "allocation_policy_config_for_storage";
       }
+
       return fetch(url, {
         headers: { "Content-Type": "application/json" },
         method: "POST",
@@ -510,23 +511,21 @@ function Client(): Client {
     },
     deleteConfiguration: (
       entity: Entity,
-      configurable_component_name: string,
+      configurable_component: ConfigurableComponent,
       key: string,
       params: object,
     ) => {
       let body: string;
       let url: string;
 
-      if (entity.type === "storage") {
-        body = JSON.stringify({ storage: entity.name, policy: configurable_component_name, key, params })
+      if (configurable_component.type === "allocation_policy") {
+        body = JSON.stringify({ storage: entity.name, policy: configurable_component.name, key, params })
         url = baseUrl + "allocation_policy_config";
       } else {
-        body = JSON.stringify({ strategy: configurable_component_name, key, params });
+        body = JSON.stringify({ strategy: configurable_component.name, key, params });
         url = baseUrl + "routing_strategy_config";
       }
 
-      console.log("paramskdjlakdelete???", params)
-
       return fetch(url, {
         headers: { "Content-Type": "application/json" },
         method: "DELETE",

snuba/admin/static/capacity_management/allocation_policy.tsx

@@ -75,7 +75,7 @@ function Configurations(props: {
     api
       .deleteConfiguration(
         entity,
-        configurable_component.name,
+        configurable_component,
         toDelete.name,
         toDelete.params
       )
@@ -95,7 +95,7 @@ function Configurations(props: {
     api
       .setConfiguration(
         entity,
-        configurable_component.name,
+        configurable_component,
         config.name,
         config.value,
         config.params

snuba/admin/static/capacity_management/types.tsx

@@ -1,17 +1,21 @@
 import { ReactNode } from "react";
 
 
-type ConfigurableComponent = {
+interface ConfigurableComponent {
+  type: string;
   name: string;
   configs: Configuration[];
   optional_config_definitions: OptionalConfigurationDefinition[];
 };
 
-type AllocationPolicy = ConfigurableComponent & {
+interface AllocationPolicy extends ConfigurableComponent {
+  type: "allocation_policy";
   query_type: string;
 };
 
-type RoutingStrategy = ConfigurableComponent;
+interface RoutingStrategy extends ConfigurableComponent {
+  type: "routing_strategy";
+}
 
 type Configuration = {
   name: string;
@@ -78,5 +82,5 @@ export {
   isStorage,
   isStrategy,
   StorageEntity,
-  StrategyEntity, ConfigurableComponent,
+  StrategyEntity, ConfigurableComponent, RoutingStrategy
 };

snuba/admin/static/cbrs/index.tsx

@@ -1,7 +1,7 @@
 import React, { useEffect, useState } from "react";
 import Client from "SnubaAdmin/api_client";
 import { Configurations } from "SnubaAdmin/capacity_management/allocation_policy";
-import { AllocationPolicy, Configuration, ConfigurableComponent } from "SnubaAdmin/capacity_management/types";
+import { AllocationPolicy, Configuration, ConfigurableComponent, RoutingStrategy } from "SnubaAdmin/capacity_management/types";
 import { CustomSelect, getParamFromStorage } from "SnubaAdmin/select";
 import { COLORS } from "SnubaAdmin/theme";
 
@@ -61,7 +61,8 @@ function CapacityBasedRoutingSystem(props: { api: Client }) {
       return <p>No strategy configurations found.</p>;
     }
 
-    const strategyComponent: ConfigurableComponent = {
+    const strategyComponent: RoutingStrategy = {
+      type: "routing_strategy",
       name: selectedStrategy,
       configs: strategyConfigs,
       optional_config_definitions: [],

snuba/admin/static/tests/capacity_management/allocation_policies.spec.tsx

@@ -8,6 +8,7 @@ import React from "react";
 
 it("should populate configs table upon render", async () => {
   let allocationPolicy: AllocationPolicy = {
+    type: "allocation_policy",
     name: "some_policy",
     configs: [
       {

snuba/admin/static/tests/capacity_management/index.spec.tsx

@@ -32,6 +32,7 @@ it("should display allocation policy configs once a storage is selected", async
   let storages = ["storage1", "storage2"];
   let allocationPolicies: AllocationPolicy[] = [
     {
+      type: "allocation_policy",
       name: "some_policy",
       configs: [
         {
@@ -46,6 +47,7 @@ it("should display allocation policy configs once a storage is selected", async
       query_type: "select",
     },
     {
+      type: "allocation_policy",
       name: "some_other_policy",
       configs: [
         {

snuba/admin/views.py

@@ -97,10 +97,10 @@
 )
 from snuba.web.rpc import RPCEndpoint, list_all_endpoint_names, run_rpc_handler
 from snuba.web.rpc.storage_routing.routing_strategies.storage_routing import (
+    CBRS_HASH,
     BaseRoutingStrategy,
 )
 from snuba.web.views import dataset_query
-from snuba.query.allocation_policies import CAPMAN_HASH
 
 logger = structlog.get_logger().bind(module=__name__)
 
@@ -990,7 +990,8 @@ def _add_policy_data(
     for policy in policies:
         data.append(
             {
-                "policy_name": policy.config_key(),
+                "type": "allocation_policy",
+                "name": policy.config_key(),
                 "configs": policy.get_current_configs(),
                 "optional_config_definitions": policy.get_optional_config_definitions_json(),
                 "query_type": query_type,
@@ -1024,7 +1025,7 @@ def get_allocation_policy_configs_of_routing_strategy(strategy_name: str) -> Res
 
     policies = BaseRoutingStrategy.get_from_name(
         strategy_name
-    ).get_allocation_policies()
+    )().get_allocation_policies()
 
     data: list[dict[str, Any]] = []
     _add_policy_data(policies, "select", data)
@@ -1041,13 +1042,15 @@ def get_routing_strategy_configs(strategy_name: str) -> Response:
 
     configs = BaseRoutingStrategy.get_from_name(strategy_name)().get_configurations()
     serialized_configs = [
-        cast(RoutingStrategyConfig, config).to_definition_dict() for config in configs.values()
+        cast(RoutingStrategyConfig, config).to_config_dict()
+        for config in configs.values()
     ]
-    print("isithere")
+    print("isithereRACHELITSHERE", serialized_configs)
     return Response(
         json.dumps(serialized_configs), 200, {"Content-Type": "application/json"}
     )
 
+
 @application.route("/routing_strategy_config", methods=["POST", "DELETE"])
 @check_tool_perms(tools=[AdminTools.CAPACITY_BASED_ROUTING_SYSTEM])
 def set_routing_strategy_config() -> Response:
@@ -1056,7 +1059,7 @@ def set_routing_strategy_config() -> Response:
     user = request.headers.get(USER_HEADER_KEY)
 
     try:
-        print("datafsdfsdf", data)
+        print("datafsdfsdf", data, data["strategy"])
         strategy, key = (data["strategy"], data["key"])
         params = data.get("params", {})
         print("params", params)
@@ -1068,10 +1071,12 @@ def set_routing_strategy_config() -> Response:
         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:
@@ -1087,9 +1092,94 @@ def set_routing_strategy_config() -> Response:
     return Response("", 200)
 
 
-@application.route("/allocation_policy_config", methods=["POST", "DELETE"])
+@application.route("/allocation_policy_config_for_strategy", methods=["POST", "DELETE"])
+@check_tool_perms(tools=[AdminTools.CAPACITY_MANAGEMENT])
+def set_allocation_policy_config_for_strategy() -> Response:
+    data = json.loads(request.data)
+    user = request.headers.get(USER_HEADER_KEY)
+
+    try:
+        strategy, key, policy_name = (data["strategy"], data["key"], data["policy"])
+        print("dkfjlskjflks", data)
+
+        params = data.get("params", {})
+
+        assert isinstance(strategy, str), "Invalid strategy"
+        assert isinstance(key, str), "Invalid key"
+        assert isinstance(params, dict), "Invalid params"
+        assert key != "", "Key cannot be empty string"
+        assert isinstance(policy_name, str), "Invalid policy name"
+        strategy = BaseRoutingStrategy.get_from_name(strategy)
+        assert strategy is not None, "Strategy not found"
+        policies = (
+            strategy().get_allocation_policies()
+            + strategy().get_delete_allocation_policies()
+        )
+        policy = next(
+            (p for p in policies if p.config_key() == policy_name),
+            None,
+        )
+        assert policy is not None, "Policy not found on strategy"
+
+    except (KeyError, AssertionError) as exc:
+        return Response(
+            json.dumps({"error": f"Invalid config: {str(exc)}"}),
+            400,
+            {"Content-Type": "application/json"},
+        )
+
+    if request.method == "DELETE":
+        policy.delete_config_value(
+            config_key=key, hash=CBRS_HASH, params=params, user=user
+        )
+        audit_log.record(
+            user or "",
+            AuditLogAction.ALLOCATION_POLICY_DELETE,
+            {
+                "storage": strategy,
+                "policy": policy.config_key(),
+                "key": key,
+            },  # todo: make audit log handle strategy in addition to storage
+            notify=True,
+        )
+        return Response("", 200)
+    elif request.method == "POST":
+        try:
+            value = data["value"]
+            assert isinstance(value, str), "Invalid value"
+            policy.set_config_value(
+                config_key=key, value=value, params=params, user=user
+            )
+            audit_log.record(
+                user or "",
+                AuditLogAction.ALLOCATION_POLICY_UPDATE,
+                {
+                    "storage": strategy,  # todo: make audit log handle strategy in addition to storage
+                    "policy": policy.config_key(),
+                    "key": key,
+                    "value": value,
+                    "params": str(params),
+                },
+                notify=True,
+            )
+            return Response("", 200)
+        except (KeyError, AssertionError) as exc:
+            return Response(
+                json.dumps({"error": f"Invalid config: {str(exc)}"}),
+                400,
+                {"Content-Type": "application/json"},
+            )
+    else:
+        return Response(
+            json.dumps({"error": "Method not allowed"}),
+            405,
+            {"Content-Type": "application/json"},
+        )
+
+
+@application.route("/allocation_policy_config_for_storage", methods=["POST", "DELETE"])
 @check_tool_perms(tools=[AdminTools.CAPACITY_MANAGEMENT])
-def set_allocation_policy_config() -> Response:
+def set_allocation_policy_config_for_storage() -> Response:
     data = json.loads(request.data)
     user = request.headers.get(USER_HEADER_KEY)
 
@@ -1115,15 +1205,16 @@ def set_allocation_policy_config() -> Response:
         assert policy is not None, "Policy not found on storage"
 
     except (KeyError, AssertionError) as exc:
-        print("4444444")
         return Response(
             json.dumps({"error": f"Invalid config: {str(exc)}"}),
             400,
             {"Content-Type": "application/json"},
         )
 
     if request.method == "DELETE":
-        policy.delete_config_value(config_key=key, hash=CAPMAN_HASH, params=params, user=user)
+        policy.delete_config_value(
+            config_key=key, hash=CAPMAN_HASH, params=params, user=user
+        )
         audit_log.record(
             user or "",
             AuditLogAction.ALLOCATION_POLICY_DELETE,
@@ -1152,7 +1243,6 @@ def set_allocation_policy_config() -> Response:
             )
             return Response("", 200)
         except (KeyError, AssertionError) as exc:
-            print("5555555")
             return Response(
                 json.dumps({"error": f"Invalid config: {str(exc)}"}),
                 400,