Skip to content

Commit

Permalink
Revert "ref(hc): Represent RPC request and response bodies as Pydanti…
Browse files Browse the repository at this point in the history
…c objects (#67248)"

This reverts commit 5b41ce7.

Co-authored-by: RyanSkonnord <26411900+RyanSkonnord@users.noreply.github.com>
  • Loading branch information
getsentry-bot and RyanSkonnord committed Apr 3, 2024
1 parent becf2a4 commit 76483b0
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 36 deletions.
21 changes: 1 addition & 20 deletions src/sentry/api/endpoints/internal/rpc.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from typing import Any

import pydantic
from pydantic.fields import Field
from rest_framework.exceptions import NotFound, ParseError, PermissionDenied, ValidationError
from rest_framework.request import Request
from rest_framework.response import Response
Expand All @@ -17,24 +16,6 @@
from sentry.utils.env import in_test_environment


class RpcMetadata(pydantic.BaseModel):
"""Metadata about an RPC request or response.
The "meta" fields appear in request or response bodies as a hook for future use.
Currently, this is always an empty object.
"""


class RpcRequestBody(pydantic.BaseModel):
meta: RpcMetadata = Field(default_factory=RpcMetadata)
args: dict[str, Any]


class RpcResponseBody(pydantic.BaseModel):
meta: RpcMetadata = Field(default_factory=RpcMetadata)
value: Any


@all_silo_endpoint
class InternalRpcServiceEndpoint(Endpoint):
publish_status = {
Expand Down Expand Up @@ -95,4 +76,4 @@ def post(self, request: Request, service_name: str, method_name: str) -> Respons
) from e
capture_exception()
raise ValidationError from e
return Response(data=RpcResponseBody(value=result).dict())
return Response(data=result)
2 changes: 1 addition & 1 deletion src/sentry/services/hybrid_cloud/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

T = TypeVar("T")

ArgumentDict = dict[str, Any]
ArgumentDict = Mapping[str, Any]

OptionValue = Any

Expand Down
16 changes: 10 additions & 6 deletions src/sentry/services/hybrid_cloud/rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from sentry.services.hybrid_cloud.sig import SerializableFunctionSignature
from sentry.silo import SiloMode, SingleProcessSiloModeState
from sentry.types.region import Region, RegionMappingNotFound
from sentry.utils import metrics
from sentry.utils import json, metrics
from sentry.utils.env import in_test_environment

if TYPE_CHECKING:
Expand Down Expand Up @@ -397,7 +397,10 @@ def result_to_dict(value: Any) -> Any:

return value

return result_to_dict(result)
return {
"meta": {}, # reserved for future use
"value": result_to_dict(result),
}


_RPC_CONTENT_CHARSET = "utf-8"
Expand Down Expand Up @@ -460,10 +463,11 @@ def _metrics_tags(self, **additional_tags: str | int) -> Mapping[str, str | int
)

def _send_to_remote_silo(self, use_test_client: bool) -> Any:
from sentry.api.endpoints.internal.rpc import RpcRequestBody

request_body = RpcRequestBody(args=self.serial_arguments)
data = request_body.json().encode(_RPC_CONTENT_CHARSET)
request_body = {
"meta": {}, # reserved for future use
"args": self.serial_arguments,
}
data = json.dumps(request_body).encode(_RPC_CONTENT_CHARSET)
signature = generate_request_signature(self.path, data)
headers = {
"Content-Type": f"application/json; charset={_RPC_CONTENT_CHARSET}",
Expand Down
10 changes: 2 additions & 8 deletions tests/sentry/api/endpoints/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from django.urls import reverse
from rest_framework.exceptions import ErrorDetail

from sentry.api.endpoints.internal.rpc import RpcRequestBody
from sentry.services.hybrid_cloud.organization import RpcUserOrganizationContext
from sentry.services.hybrid_cloud.rpc import generate_request_signature
from sentry.testutils.cases import APITestCase
Expand Down Expand Up @@ -46,36 +45,31 @@ def _send_post_request(self, path, data):

def test_missing_authentication(self):
path = self._get_path("organization", "get_organization_by_id")
data: dict[str, Any] = {"meta": {}, "args": {"organization_id": self.organization.id}}
assert data == RpcRequestBody(args={"organization_id": self.organization.id}).dict()
data: dict[str, Any] = {"args": {}, "meta": {"organization_id": self.organization.id}}
response = self.client.post(path, data=data)
assert response.status_code == 403

def test_invalid_authentication(self):
path = self._get_path("organization", "get_organization_by_id")
data: dict[str, Any] = {"meta": {}, "args": {"organization_id": self.organization.id}}
assert data == RpcRequestBody(args={"organization_id": self.organization.id}).dict()
data: dict[str, Any] = {"args": {}, "meta": {"organization_id": self.organization.id}}
response = self.client.post(path, data=data, HTTP_AUTHORIZATION="rpcsignature trash")
assert response.status_code == 401

def test_bad_service_name(self):
path = self._get_path("not_a_service", "not_a_method")
data: dict[str, Any] = {"args": {}, "meta": {}}
assert data == RpcRequestBody(args={}).dict()
response = self._send_post_request(path, data)
assert response.status_code == 404

def test_bad_method_name(self):
path = self._get_path("user", "not_a_method")
data: dict[str, Any] = {"args": {}, "meta": {}}
assert data == RpcRequestBody(args={}).dict()
response = self._send_post_request(path, data)
assert response.status_code == 404

def test_no_body(self):
path = self._get_path("organization", "get_organization_by_id")
data: dict[str, Any] = {"args": {}, "meta": {}}
assert data == RpcRequestBody(args={}).dict()
response = self._send_post_request(path, data)
assert response.status_code == 400
assert response.data == {
Expand Down
3 changes: 2 additions & 1 deletion tests/sentry/hybridcloud/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ def test_dispatch_to_local_service_list_result(self):
args = {"organization_ids": [organization.id]}
with assume_test_silo_mode(SiloMode.CONTROL):
service = AuthService.create_delegation()
result = dispatch_to_local_service(service.key, "get_org_auth_config", args)
response = dispatch_to_local_service(service.key, "get_org_auth_config", args)
result = response["value"]
assert len(result) == 1
assert result[0]["organization_id"] == organization.id

Expand Down

0 comments on commit 76483b0

Please sign in to comment.