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.

Sign up for GitHub

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

chore(hybridcloud) Enable stricter types on more modules #70818

Merged
merged 3 commits into from
May 14, 2024
Merged

chore(hybridcloud) Enable stricter types on more modules #70818

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9dd0c9e
chore(hybridcloud) Enable stricter types on more modules
markstory May 13, 2024
046dd70
Fix type
markstory May 14, 2024
422da91
Merge branch 'master' into chore-add-strict-types
markstory May 14, 2024
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
5 changes: 4 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,8 @@ module = [
"sentry.tasks.commit_context",
"sentry.tasks.on_demand_metrics",
"sentry.tasks.reprocessing2",
"sentry.utils.actor",
"sentry.types.actor",
"sentry.types.region",
"sentry.utils.arroyo",
"sentry.utils.assets",
"sentry.utils.audit",
Expand Down Expand Up @@ -699,6 +700,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from contextlib import contextmanager
from unittest.mock import patch
from typing import Any
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) -> Any:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Loading