Skip to content

Commit

Permalink
chore(hybridcloud) Enable stricter types on more modules (#70818)
Browse files Browse the repository at this point in the history
Move a few more modules into the stricter allow list.
  • Loading branch information
markstory committed May 14, 2024
1 parent 7d90ddb commit e245806
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 25 deletions.
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,7 @@ module = [
"sentry.tasks.on_demand_metrics",
"sentry.tasks.reprocessing2",
"sentry.types.actor",
"sentry.types.region",
"sentry.utils.arroyo",
"sentry.utils.assets",
"sentry.utils.audit",
Expand Down Expand Up @@ -697,6 +698,8 @@ module = [
"tests.sentry.relay.config.test_metric_extraction",
"tests.sentry.services.hybrid_cloud.*",
"tests.sentry.tasks.test_on_demand_metrics",
"tests.sentry.types.test_actor",
"tests.sentry.types.test_region",
"tools.*",
]
disallow_any_generics = true
Expand Down
16 changes: 8 additions & 8 deletions tests/sentry/types/test_actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


@django_db_all(transaction=True)
def test_many_from_object_users():
def test_many_from_object_users() -> None:
users = [Factories.create_user(), Factories.create_user()]
actors = Actor.many_from_object(users)
assert len(actors) == len(users)
Expand All @@ -25,7 +25,7 @@ def test_many_from_object_users():


@django_db_all(transaction=True)
def test_from_identifier():
def test_from_identifier() -> None:
user = Factories.create_user()
org = Factories.create_organization(owner=user)
team = Factories.create_team(organization=org)
Expand Down Expand Up @@ -67,7 +67,7 @@ def test_from_identifier():
assert not actor.is_user


def test_from_id():
def test_from_id() -> None:
actor = Actor.from_id(team_id=1)
assert actor
assert actor.id == 1
Expand All @@ -85,7 +85,7 @@ def test_from_id():


@django_db_all(transaction=True)
def test_many_from_object_rpc_users():
def test_many_from_object_rpc_users() -> None:
orm_users = [Factories.create_user(), Factories.create_user()]
user_ids = [u.id for u in orm_users]
rpc_users = user_service.get_many(filter={"user_ids": user_ids})
Expand All @@ -101,7 +101,7 @@ def test_many_from_object_rpc_users():


@django_db_all(transaction=True)
def test_many_from_object_teams():
def test_many_from_object_teams() -> None:
organization = Factories.create_organization()
teams = [
Factories.create_team(organization=organization),
Expand All @@ -121,7 +121,7 @@ def test_many_from_object_teams():


@django_db_all(transaction=True)
def test_many_from_object_mixed():
def test_many_from_object_mixed() -> None:
organization = Factories.create_organization()
teams = [
Factories.create_team(organization=organization),
Expand All @@ -141,7 +141,7 @@ def test_many_from_object_mixed():


@django_db_all(transaction=True)
def test_resolve_many():
def test_resolve_many() -> None:
organization = Factories.create_organization()
team_one = Factories.create_team(organization=organization)
team_two = Factories.create_team(organization=organization)
Expand All @@ -167,7 +167,7 @@ def test_resolve_many():


@django_db_all(transaction=True)
def test_parse_and_validate_actor():
def test_parse_and_validate_actor() -> None:
user = Factories.create_user()
other_user = Factories.create_user()
org = Factories.create_organization(owner=user)
Expand Down
35 changes: 18 additions & 17 deletions tests/sentry/types/test_region.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections.abc import Generator
from contextlib import contextmanager
from unittest.mock import patch
from unittest.mock import MagicMock, patch

import pytest
from django.db import router
Expand Down Expand Up @@ -68,11 +69,11 @@ class RegionDirectoryTest(TestCase):

@staticmethod
@contextmanager
def _in_global_state(directory: RegionDirectory):
def _in_global_state(directory: RegionDirectory) -> Generator[None, None, None]:
with get_test_env_directory().swap_state(tuple(directory.regions)):
yield

def test_region_config_parsing_in_monolith(self):
def test_region_config_parsing_in_monolith(self) -> None:
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(self._INPUTS)
assert directory.regions == frozenset(self._EXPECTED_OUTPUTS)
Expand All @@ -84,7 +85,7 @@ def test_region_config_parsing_in_monolith(self):
with pytest.raises(RegionResolutionError):
get_region_by_name("nowhere")

def test_region_config_parsing_in_control(self):
def test_region_config_parsing_in_control(self) -> None:
with (
override_settings(SILO_MODE=SiloMode.CONTROL),
override_settings(SENTRY_MONOLITH_REGION="us"),
Expand All @@ -93,19 +94,19 @@ def test_region_config_parsing_in_control(self):
assert directory.regions == frozenset(self._EXPECTED_OUTPUTS)

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_json_config_injection(self):
def test_json_config_injection(self) -> None:
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(json.dumps(self._INPUTS))
assert directory.regions == frozenset(self._EXPECTED_OUTPUTS)

@override_settings(SILO_MODE=SiloMode.REGION, SENTRY_REGION="us")
def test_get_local_region(self):
def test_get_local_region(self) -> None:
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(self._INPUTS)
with self._in_global_state(directory):
assert get_local_region() == self._EXPECTED_OUTPUTS[0]

def test_get_generated_monolith_region(self):
def test_get_generated_monolith_region(self) -> None:
with (
override_settings(SILO_MODE=SiloMode.MONOLITH, SENTRY_MONOLITH_REGION="defaultland"),
self._in_global_state(load_from_config(())),
Expand All @@ -117,7 +118,7 @@ def test_get_generated_monolith_region(self):

@override_settings(SILO_MODE=SiloMode.CONTROL)
@unguarded_write(using=router.db_for_write(OrganizationMapping))
def test_get_region_for_organization(self):
def test_get_region_for_organization(self) -> None:
mapping = OrganizationMapping.objects.get(slug=self.organization.slug)
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(self._INPUTS)
Expand All @@ -140,11 +141,11 @@ def test_get_region_for_organization(self):
# OrganizationMapping does not exist
get_region_for_organization(self.organization.slug)

def test_validate_region(self):
def test_validate_region(self) -> None:
for region in self._EXPECTED_OUTPUTS:
region.validate()

def test_region_to_url(self):
def test_region_to_url(self) -> None:
region = Region("us", 1, "http://192.168.1.99", RegionCategory.MULTI_TENANT)
with override_settings(SILO_MODE=SiloMode.REGION, SENTRY_REGION="us"):
assert region.to_url("/avatar/abcdef/") == "http://us.testserver/avatar/abcdef/"
Expand All @@ -154,20 +155,20 @@ def test_region_to_url(self):
assert region.to_url("/avatar/abcdef/") == "http://testserver/avatar/abcdef/"

@patch("sentry.types.region.sentry_sdk")
def test_invalid_config(self, sentry_sdk_mock):
def test_invalid_config(self, sentry_sdk_mock: MagicMock) -> None:
region_config = ["invalid"]
assert sentry_sdk_mock.capture_exception.call_count == 0
with pytest.raises(RegionConfigurationError):
load_from_config(region_config)
assert sentry_sdk_mock.capture_exception.call_count == 1

def test_invalid_historic_region_setting(self):
def test_invalid_historic_region_setting(self) -> None:
with pytest.raises(RegionConfigurationError):
with override_settings(SENTRY_MONOLITH_REGION="nonexistent"):
load_from_config(self._INPUTS)

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_find_regions_for_user(self):
def test_find_regions_for_user(self) -> None:
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(self._INPUTS)
with self._in_global_state(directory):
Expand All @@ -189,23 +190,23 @@ def test_find_regions_for_user(self):
find_regions_for_user(user_id=user.id)

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_find_all_region_names(self):
def test_find_all_region_names(self) -> None:
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(self._INPUTS)
with self._in_global_state(directory):
result = find_all_region_names()
assert set(result) == {"us", "eu", "acme"}

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_find_all_multitenant_region_names(self):
def test_find_all_multitenant_region_names(self) -> None:
with override_settings(SENTRY_MONOLITH_REGION="us"):
directory = load_from_config(self._INPUTS)
with self._in_global_state(directory):
result = find_all_multitenant_region_names()
assert set(result) == {"us", "eu"}

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_find_all_multitenant_region_names_non_visible(self):
def test_find_all_multitenant_region_names_non_visible(self) -> None:
inputs = [
*self._INPUTS,
{
Expand All @@ -223,7 +224,7 @@ def test_find_all_multitenant_region_names_non_visible(self):
assert set(result) == {"us", "eu"}

@override_settings(SILO_MODE=SiloMode.CONTROL)
def test_subdomain_is_region(self):
def test_subdomain_is_region(self) -> None:
regions = [
Region("us", 1, "http://us.testserver", RegionCategory.MULTI_TENANT),
]
Expand Down

0 comments on commit e245806

Please sign in to comment.