Skip to content

Commit

Permalink
🛠️ apply pre-commit fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
getsantry[bot] authored Feb 8, 2025
1 parent 2223305 commit 9ecf8c9
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 33 deletions.
28 changes: 18 additions & 10 deletions src/sentry/tempest/tasks.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import logging
import json
import logging

import requests
import sentry_sdk
from django.db import transaction
from django.conf import settings
from django.db import transaction
from requests import Response

from sentry import options
Expand All @@ -17,17 +17,22 @@

logger = logging.getLogger(__name__)


class TempestError(Exception):
"""Base exception for Tempest-related errors."""

pass


class TempestAPIError(TempestError):
"""Exception raised for Tempest API errors."""

def __init__(self, message: str, status_code: int, response_body: str):
self.status_code = status_code
self.response_body = response_body
super().__init__(message)


def validate_tempest_response(response: Response, required_fields: list[str]) -> dict:
"""Validates a Tempest API response and returns the parsed JSON data."""
if response.status_code >= 500:
Expand All @@ -36,14 +41,14 @@ def validate_tempest_response(response: Response, required_fields: list[str]) ->
response.status_code,
response.text,
)

if response.status_code >= 400:
raise TempestAPIError(
f"Tempest request error: {response.status_code}",
response.status_code,
response.text,
)

try:
result = response.json()
except ValueError as e:
Expand All @@ -52,7 +57,7 @@ def validate_tempest_response(response: Response, required_fields: list[str]) ->
response.status_code,
response.text,
) from e

if "error" in result:
error_type = result["error"].get("type", "unknown")
error_message = result["error"].get("message", "Unknown error")
Expand All @@ -61,17 +66,18 @@ def validate_tempest_response(response: Response, required_fields: list[str]) ->
response.status_code,
response.text,
)

missing_fields = [field for field in required_fields if field not in result]
if missing_fields:
raise TempestAPIError(
f"Missing required fields in response: {', '.join(missing_fields)}",
response.status_code,
response.text,
)

return result


@instrumented_task(
name="sentry.tempest.tasks.poll_tempest",
queue="tempest",
Expand Down Expand Up @@ -211,15 +217,17 @@ def poll_tempest_crashes(credentials_id: int, **kwargs) -> None:

try:
result = validate_tempest_response(response, required_fields=["latest_id"])

# Use select_for_update to prevent race conditions
with transaction.atomic():
credentials = TempestCredentials.objects.select_for_update().get(id=credentials_id)
credentials.latest_fetched_item_id = result["latest_id"]
credentials.message = "" # Clear any previous error messages
credentials.message_type = MessageType.INFO
credentials.save(update_fields=["latest_fetched_item_id", "message", "message_type"])

credentials.save(
update_fields=["latest_fetched_item_id", "message", "message_type"]
)

except TempestAPIError as e:
credentials.message = f"Error fetching crashes: {str(e)}"
credentials.message_type = MessageType.ERROR
Expand Down
12 changes: 7 additions & 5 deletions src/sentry/tempest/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
import json

from requests import Response


def create_mock_response(status_code: int, body: dict = None, is_json: bool = True) -> Response:
"""
Creates a mock Response object for testing.
Args:
status_code: HTTP status code to return
body: Response body (dict for JSON responses)
is_json: Whether the response should be treated as JSON
"""
response = Response()
response.status_code = status_code

if body is None:
body = {}

if is_json:
response._content = json.dumps(body).encode()
else:
response._content = str(body).encode()
return response

return response
31 changes: 13 additions & 18 deletions tests/sentry/tempest/test_tempest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@

from sentry.models.projectkey import ProjectKey, UseCase
from sentry.tempest.models import MessageType
from sentry.tempest.tasks import (fetch_latest_item_id, poll_tempest,
poll_tempest_crashes, TempestError, TempestAPIError)
from sentry.tempest.tasks import (
TempestAPIError,
TempestError,
fetch_latest_item_id,
poll_tempest,
poll_tempest_crashes,
)
from sentry.tempest.test_utils import create_mock_response
from sentry.testutils.cases import TestCase

Expand Down Expand Up @@ -221,17 +226,14 @@ def test_poll_tempest_crashes_http_500_retry(self, mock_fetch):
self.credentials.save()

# Simulate a 500 error response
mock_fetch.return_value = create_mock_response(
500,
{"error": "Internal Server Error"}
)
mock_fetch.return_value = create_mock_response(500, {"error": "Internal Server Error"})

# Should raise TempestAPIError to trigger retry
with self.assertRaises(TempestAPIError) as cm:
poll_tempest_crashes(self.credentials.id)

self.assertEqual(cm.exception.status_code, 500)

# Verify credentials were updated with error message
self.credentials.refresh_from_db()
assert self.credentials.message.startswith("Error fetching crashes")
Expand All @@ -245,17 +247,13 @@ def test_poll_tempest_crashes_invalid_json(self, mock_fetch):
self.credentials.save()

# Return invalid JSON response
mock_fetch.return_value = create_mock_response(
200,
"Invalid JSON",
is_json=False
)
mock_fetch.return_value = create_mock_response(200, "Invalid JSON", is_json=False)

with self.assertRaises(TempestAPIError) as cm:
poll_tempest_crashes(self.credentials.id)

assert "Invalid JSON response" in str(cm.exception)

# Verify state preserved
self.credentials.refresh_from_db()
assert self.credentials.latest_fetched_item_id == "42"
Expand All @@ -269,17 +267,14 @@ def test_poll_tempest_crashes_missing_fields(self, mock_fetch):
self.credentials.save()

# Return response missing latest_id
mock_fetch.return_value = create_mock_response(
200,
{"data": "some data but no latest_id"}
)
mock_fetch.return_value = create_mock_response(200, {"data": "some data but no latest_id"})

with self.assertRaises(TempestAPIError) as cm:
poll_tempest_crashes(self.credentials.id)

assert "Missing required fields" in str(cm.exception)
assert "latest_id" in str(cm.exception)

# Verify state preserved
self.credentials.refresh_from_db()
assert self.credentials.latest_fetched_item_id == "42"
Expand Down

0 comments on commit 9ecf8c9

Please sign in to comment.