Skip to content
Merged
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
3 changes: 0 additions & 3 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -4133,6 +4133,3 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]:
# the region API URL template is set to the ngrok host.
SENTRY_OPTIONS["system.region-api-url-template"] = f"https://{{region}}.{ngrok_host}"
SENTRY_FEATURES["system:multi-region"] = True

if IS_DEV:
SENTRY_OPTIONS["taskworker.enabled"] = True
2 changes: 1 addition & 1 deletion src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -3340,7 +3340,7 @@
# Taskbroker flags
register(
"taskworker.enabled",
default=False,
default=True,
flags=FLAG_AUTOMATOR_MODIFIABLE,
Comment on lines +3343 to 3344
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential bug: Changing the default of taskworker.enabled to True will break self-hosted deployments, which lack the required Kafka infrastructure and will fail to process background tasks.
  • Description: Changing the default for taskworker.enabled to True routes background tasks to the taskworker system. This system relies on Kafka, as shown by topic definitions in src/sentry/conf/types/kafka_definition.py and producer logic in src/sentry/taskworker/registry.py. Self-hosted deployments are configured with Redis/RabbitMQ for Celery and do not have Kafka infrastructure. The change lacks a corresponding override in self-hosted/sentry.conf.py to keep taskworkers disabled. As a result, self-hosted instances will attempt to send tasks to non-existent Kafka brokers, causing background job processing to fail and breaking core Sentry functionality.

  • Suggested fix: Add SENTRY_OPTIONS["taskworker.enabled"] = False to self-hosted/sentry.conf.py. This will maintain the existing Celery-based task processing for self-hosted instances and prevent breakages until they are prepared to migrate to the new taskworker system.
    severity: 0.92, confidence: 0.98

Did we get this right? 👍 / 👎 to inform future reviews.

)
register(
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/relocation/tasks/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,8 @@ def uploading_start(uuid: str, replying_region_name: str | None, org_slug: str |
# reasonable amount of time, go ahead and fail the relocation.
cross_region_export_timeout_check.apply_async(
args=[uuid],
countdown=int(CROSS_REGION_EXPORT_TIMEOUT.total_seconds()),
# In tests we mock this timeout to be a negative value.
countdown=max(int(CROSS_REGION_EXPORT_TIMEOUT.total_seconds()), 0),
)
return

Expand Down
192 changes: 0 additions & 192 deletions tests/sentry/api/endpoints/test_project_codeowners_details.py

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from datetime import datetime, timezone
from unittest import mock
from unittest.mock import MagicMock, patch

Expand All @@ -9,6 +8,7 @@
from sentry.models.projectcodeowners import ProjectCodeOwners
from sentry.testutils.cases import APITestCase
from sentry.testutils.helpers.analytics import assert_last_analytics_event
from sentry.testutils.helpers.datetime import freeze_time


class ProjectCodeOwnersDetailsEndpointTestCase(APITestCase):
Expand Down Expand Up @@ -63,12 +63,10 @@ def test_basic_delete(self) -> None:
assert response.status_code == 204
assert not ProjectCodeOwners.objects.filter(id=str(self.codeowners.id)).exists()

@patch("django.utils.timezone.now")
def test_basic_update(self, mock_timezone_now: MagicMock) -> None:
@freeze_time("2023-10-03 00:00:00")
def test_basic_update(self) -> None:
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests were also duplicated in tests/sentry/api/endpoints/test_project_codeowners_details.py. It looks like the old test module was left behind during #97916

self.create_external_team(external_name="@getsentry/frontend", integration=self.integration)
self.create_external_team(external_name="@getsentry/docs", integration=self.integration)
date = datetime(2023, 10, 3, tzinfo=timezone.utc)
mock_timezone_now.return_value = date
raw = "\n# cool stuff comment\n*.js @getsentry/frontend @NisanthanNanthakumar\n# good comment\n\n\n docs/* @getsentry/docs @getsentry/ecosystem\n\n"
data = {
"raw": raw,
Expand All @@ -89,7 +87,7 @@ def test_basic_update(self, mock_timezone_now: MagicMock) -> None:
assert response.data["id"] == str(self.codeowners.id)
assert response.data["raw"] == raw.strip()
codeowner = ProjectCodeOwners.objects.filter(id=self.codeowners.id)[0]
assert codeowner.date_updated == date
assert codeowner.date_updated.strftime("%Y-%m-%d %H:%M:%S") == "2023-10-03 00:00:00"

def test_wrong_codeowners_id(self) -> None:
self.url = reverse(
Expand Down
12 changes: 4 additions & 8 deletions tests/sentry/tasks/test_code_owners.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from datetime import datetime, timezone
from unittest.mock import MagicMock, patch

from sentry.integrations.models.external_actor import ExternalActor
Expand All @@ -9,6 +8,7 @@
from sentry.models.repository import Repository
from sentry.tasks.codeowners import code_owners_auto_sync, update_code_owners_schema
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers.datetime import freeze_time

LATEST_GITHUB_CODEOWNERS = {
"filepath": "CODEOWNERS",
Expand Down Expand Up @@ -82,14 +82,12 @@ def test_simple(self) -> None:

assert code_owners.schema == {"$version": 1, "rules": []}

@patch("django.utils.timezone.now")
@freeze_time("2023-01-01 00:00:00")
@patch(
"sentry.integrations.github.integration.GitHubIntegration.get_codeowner_file",
return_value=LATEST_GITHUB_CODEOWNERS,
)
def test_codeowners_auto_sync_successful(
self, mock_get_codeowner_file: MagicMock, mock_timezone_now: MagicMock
) -> None:
def test_codeowners_auto_sync_successful(self, mock_get_codeowner_file: MagicMock) -> None:
code_owners = ProjectCodeOwners.objects.get(id=self.code_owners.id)
assert code_owners.raw == self.data["raw"]

Expand All @@ -108,8 +106,6 @@ def test_codeowners_auto_sync_successful(
filename=".github/CODEOWNERS",
type="A",
)
mock_now = datetime(2023, 1, 1, 0, 0, tzinfo=timezone.utc)
mock_timezone_now.return_value = mock_now
code_owners_auto_sync(commit.id)

code_owners = ProjectCodeOwners.objects.get(id=self.code_owners.id)
Expand All @@ -132,7 +128,7 @@ def test_codeowners_auto_sync_successful(
},
],
}
assert code_owners.date_updated == mock_now
assert code_owners.date_updated.strftime("%Y-%m-%d %H:%M:%S") == "2023-01-01 00:00:00"

@patch(
"sentry.integrations.github.integration.GitHubIntegration.get_codeowner_file",
Expand Down
4 changes: 2 additions & 2 deletions tests/sentry/tasks/test_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ def test_debounce(
):
tasks = []

def apply_async(args, kwargs):
def signal_send(self, task, args, kwargs):
assert not args
tasks.append(kwargs)

with mock.patch("sentry.tasks.relay.build_project_config.apply_async", apply_async):
with mock.patch("sentry.taskworker.task.Task._signal_send", signal_send):
schedule_build_project_config(public_key=default_projectkey.public_key)
schedule_build_project_config(public_key=default_projectkey.public_key)

Expand Down
5 changes: 4 additions & 1 deletion tests/sentry/workflow_engine/processors/test_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,10 @@ def test_doesnt_send_metric(self) -> None:

with mock.patch("sentry.utils.metrics.incr") as mock_incr:
process_detectors(data_packet, [detector])
mock_incr.assert_not_called()
calls = mock_incr.call_args_list
# We can have background threads emitting metrics as tasks are scheduled
filtered_calls = list(filter(lambda c: "taskworker" not in c.args[0], calls))
assert len(filtered_calls) == 0


@django_db_all
Expand Down
Loading