Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE - Api Gateway hacks #40288

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/api_gateway/api_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
def _request_should_be_proxied(request: Request, view_func, view_kwargs) -> bool:
view_class = getattr(view_func, "view_class", None)
current_silo_mode = SiloMode.get_current_mode()
if view_class is not None:
if current_silo_mode != SiloMode.MONOLITH and view_class is not None:
endpoint_silo_limit = getattr(view_class, "silo_limit", None)
if endpoint_silo_limit is not None:
endpoint_silo_set = endpoint_silo_limit.modes
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/api_gateway/proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Utilities related to proxying a request to a region silo
"""

from wsgiref.util import is_hop_by_hop

from django.conf import settings
from django.http import StreamingHttpResponse
from requests import Response as ExternalResponse
Expand All @@ -26,11 +28,16 @@ def stream_response(): # type: ignore
streamed_response = StreamingHttpResponse(
streaming_content=stream_response(),
status=response.status_code,
content_type=response.headers.pop("Content-Type"),
content_type=response.headers.pop("Content-Type", None),
)
# Add Headers to response
marked_for_deletion = []
for header, value in response.headers.items():
streamed_response[header] = value
if is_hop_by_hop(header):
marked_for_deletion.append(header)
for header in marked_for_deletion:
del streamed_response[header]
return streamed_response


Expand Down
1 change: 1 addition & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ def env(key, default="", type=None):
# response modifying middleware reset the Content-Length header.
# This is because CommonMiddleware Sets the Content-Length header for non-streaming responses.
MIDDLEWARE = (
"sentry.middleware.api_gateway.ApiGatewayMiddleware",
"sentry.middleware.health.HealthCheck",
"sentry.middleware.security.SecurityHeadersMiddleware",
"sentry.middleware.env.SentryEnvMiddleware",
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/models/apitoken.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
BaseManager,
FlexibleForeignKey,
Model,
region_silo_only_model,
control_silo_only_model,
sane_repr,
)
from sentry.models.apiscopes import HasApiScopes
Expand All @@ -25,7 +25,7 @@ def generate_token():
return uuid4().hex + uuid4().hex


@region_silo_only_model
@control_silo_only_model
class ApiToken(Model, HasApiScopes):
__include_in_export__ = True

Expand Down
4 changes: 2 additions & 2 deletions src/sentry/models/options/user_option.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.conf import settings
from django.db import models

from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model, sane_repr
from sentry.db.models import FlexibleForeignKey, Model, control_silo_only_model, sane_repr
from sentry.db.models.fields import PickledObjectField
from sentry.db.models.manager import OptionManager, Value

Expand Down Expand Up @@ -120,7 +120,7 @@ def post_delete(self, instance: UserOption, **kwargs: Any) -> None:

# TODO(dcramer): the NULL UNIQUE constraint here isn't valid, and instead has to
# be manually replaced in the database. We should restructure this model.
@region_silo_only_model
@control_silo_only_model
class UserOption(Model): # type: ignore
"""
User options apply only to a user, and optionally a project OR an organization.
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
BaseModel,
BoundedAutoField,
FlexibleForeignKey,
region_silo_only_model,
control_silo_only_model,
sane_repr,
)
from sentry.models import LostPasswordHash
Expand Down Expand Up @@ -115,7 +115,7 @@ def get_for_email(self, email: str, case_sensitive: bool = True) -> Sequence["Us
return self.filter(emails__is_verified=True, is_active=True, **kwargs)


@region_silo_only_model
@control_silo_only_model
class User(BaseModel, AbstractBaseUser):
__include_in_export__ = True

Expand Down
15 changes: 14 additions & 1 deletion src/sentry/types/region.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ def get_region_for_organization(organization: Organization) -> Region:
Raises RegionContextError if this Sentry platform instance is configured to
run only in monolith mode.
"""
if organization is None:
# KLUDGE
return Region(
name="region1",
id=1,
address="http://127.0.0.1:9001",
category=RegionCategory.MULTI_TENANT,
)

mapping = _load_global_regions()

if not mapping.regions:
Expand All @@ -116,7 +125,11 @@ def get_region_for_organization(organization: Organization) -> Region:
# Backend representation to be determined. If you are working on code
# that depends on this method, you can mock it out in unit tests or
# temporarily hard-code a placeholder.
raise NotImplementedError

# KLUDGE
return Region(
name="region1", id=1, address="127.0.0.1:9001", category=RegionCategory.MULTI_TENANT
)


def get_local_region() -> Region:
Expand Down