Skip to content

Commit

Permalink
Addressed first bunch of feedbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
tobias-wilfert committed Jan 7, 2025
1 parent e2e4f71 commit 6fa4e0c
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 12 deletions.
1 change: 0 additions & 1 deletion src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,6 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str:
"dynamicsampling",
routing_key="dynamicsampling",
),
Queue("tempest", routing_key="tempest"),
Queue("incidents", routing_key="incidents"),
Queue("incident_snapshots", routing_key="incident_snapshots"),
Queue("incidents", routing_key="incidents"),
Expand Down
26 changes: 18 additions & 8 deletions src/sentry/tempest/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from sentry import http
from sentry.models.projectkey import ProjectKey, UseCase
from sentry.silo.base import SiloMode
from sentry.tasks.base import instrumented_task
from sentry.tempest.models import TempestCredentials
from sentry.utils import json
Expand All @@ -16,6 +17,9 @@
@instrumented_task(
name="sentry.tempest.tasks.poll_tempest",
queue="tempest",
silo_mode=SiloMode.REGION,
soft_time_limit=4 * 60,
time_limit=4 * 60 + 5,
)
def poll_tempest(**kwargs):
# FIXME: Once we have more traffic this needs to be done smarter.
Expand All @@ -29,6 +33,9 @@ def poll_tempest(**kwargs):
@instrumented_task(
name="sentry.tempest.tasks.fetch_latest_item_id",
queue="tempest",
silo_mode=SiloMode.REGION,
soft_time_limit=1 * 60,
time_limit=1 * 60 + 5,
)
def fetch_latest_item_id(credentials_id: int) -> None:
# FIXME: Try catch this later
Expand Down Expand Up @@ -72,8 +79,11 @@ def fetch_latest_item_id(credentials_id: int) -> None:


@instrumented_task(
name="sentry.tempest.tasks.poll_tempest",
name="sentry.tempest.tasks.poll_tempest_crashes",
queue="tempest",
silo_mode=SiloMode.REGION,
soft_time_limit=4 * 60,
time_limit=4 * 60 + 5,
)
def poll_tempest_crashes(credentials_id: int) -> None:
# FIXME: Try catch this later
Expand All @@ -83,16 +93,16 @@ def poll_tempest_crashes(credentials_id: int) -> None:
client_id = credentials.client_id

try:
response_text = fetch_crashes_from_tempest(
# This does generate a dsn not sure if it will work in the grand scheme of things though.
dsn = ProjectKey.objects.get_or_create(
use_case=UseCase.TEMPEST, project=credentials.project
)[0].get_dsn()
response_text = fetch_items_from_tempest(
org_id=org_id,
project_id=project_id,
client_id=client_id,
client_secret=credentials.client_secret,
dsn=ProjectKey.objects.get_or_create(
use_case=UseCase.TEMPEST, project=credentials.project
)[
0
].get_dsn(), # This does generate a dsn not sure if it will work in the grand scheme of things though.
dsn=dsn,
offset=int(
credentials.latest_fetched_item_id
), # Need to convert here because it is a char in the DB
Expand Down Expand Up @@ -146,7 +156,7 @@ def fetch_latest_id_from_tempest(
return response.text


def fetch_crashes_from_tempest(
def fetch_items_from_tempest(
org_id: int,
project_id: int,
client_id: str,
Expand Down
9 changes: 6 additions & 3 deletions tests/sentry/tempest/test_tempest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from unittest.mock import patch

from sentry.tempest.models import MessageType
from sentry.tempest.tasks import fetch_latest_item_id, poll_tempest_crashes
from sentry.testutils.cases import TestCase
from sentry.utils import json
Expand Down Expand Up @@ -30,6 +31,7 @@ def test_fetch_latest_item_id_error(self, mock_fetch):

self.credentials.refresh_from_db()
assert self.credentials.message == "Seems like the provided credentials are invalid"
assert self.credentials.message_type == MessageType.ERROR
assert self.credentials.latest_fetched_item_id is None

@patch("sentry.tempest.tasks.fetch_latest_id_from_tempest")
Expand All @@ -40,6 +42,7 @@ def test_fetch_latest_item_id_ip_not_allowlisted(self, mock_fetch):

self.credentials.refresh_from_db()
assert self.credentials.message == "Seems like our IP is not allow-listed"
assert self.credentials.message_type == MessageType.ERROR
assert self.credentials.latest_fetched_item_id is None

@patch("sentry.tempest.tasks.fetch_latest_id_from_tempest")
Expand All @@ -66,7 +69,7 @@ def test_fetch_latest_item_id_connection_error(self, mock_fetch):
mock_fetch.assert_called_once()
assert "Fetching the latest item id failed." in cm.output[0]

@patch("sentry.tempest.tasks.fetch_crashes_from_tempest")
@patch("sentry.tempest.tasks.fetch_items_from_tempest")
def test_poll_tempest_crashes_task(self, mock_fetch):
mock_fetch.return_value = json.dumps({"latest_id": 20002})

Expand All @@ -80,7 +83,7 @@ def test_poll_tempest_crashes_task(self, mock_fetch):
assert self.credentials.latest_fetched_item_id == "20002"
mock_fetch.assert_called_once()

@patch("sentry.tempest.tasks.fetch_crashes_from_tempest")
@patch("sentry.tempest.tasks.fetch_items_from_tempest")
def test_poll_tempest_crashes_invalid_json(self, mock_fetch):
mock_fetch.return_value = "not valid json"

Expand All @@ -97,7 +100,7 @@ def test_poll_tempest_crashes_invalid_json(self, mock_fetch):
mock_fetch.assert_called_once()
assert "Fetching the crashes failed." in cm.output[0]

@patch("sentry.tempest.tasks.fetch_crashes_from_tempest")
@patch("sentry.tempest.tasks.fetch_items_from_tempest")
def test_poll_tempest_crashes_error(self, mock_fetch):
mock_fetch.side_effect = Exception("Connection error")

Expand Down

0 comments on commit 6fa4e0c

Please sign in to comment.