-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ref(auto_source_config): Move logic around #88251
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
|
||
from .constants import METRIC_PREFIX | ||
from .integration_utils import InstallationNotFoundError, get_installation | ||
from .utils import PlatformConfig | ||
from .utils.platform import PlatformConfig | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -74,7 +74,7 @@ def derive_code_mappings( | |
trees_helper = CodeMappingTreesHelper(trees) | ||
try: | ||
frame_filename = FrameInfo(frame, platform) | ||
return trees_helper.list_file_matches(frame_filename) | ||
return trees_helper.get_file_and_repo_matches(frame_filename) | ||
except NeedsExtension: | ||
logger.warning("Needs extension: %s", frame.get("filename")) | ||
|
||
|
@@ -190,7 +190,7 @@ def generate_code_mappings( | |
|
||
return list(self.code_mappings.values()) | ||
|
||
def list_file_matches(self, frame_filename: FrameInfo) -> list[dict[str, str]]: | ||
def get_file_and_repo_matches(self, frame_filename: FrameInfo) -> list[dict[str, str]]: | ||
"""List all the files in a repo that match the frame_filename""" | ||
file_matches = [] | ||
for repo_full_name in self.trees.keys(): | ||
|
@@ -425,33 +425,31 @@ def convert_stacktrace_frame_path_to_source_path( | |
|
||
def create_code_mapping( | ||
organization: Organization, | ||
code_mapping: CodeMapping, | ||
project: Project, | ||
stacktrace_root: str, | ||
source_path: str, | ||
repo_name: str, | ||
branch: str, | ||
) -> RepositoryProjectPathConfig: | ||
installation = get_installation(organization) | ||
# It helps with typing since org_integration can be None | ||
if not installation.org_integration: | ||
raise InstallationNotFoundError | ||
|
||
repository, _ = Repository.objects.get_or_create( | ||
name=repo_name, | ||
name=code_mapping.repo.name, | ||
organization_id=organization.id, | ||
defaults={"integration_id": installation.model.id}, | ||
) | ||
new_code_mapping, _ = RepositoryProjectPathConfig.objects.update_or_create( | ||
project=project, | ||
stack_root=stacktrace_root, | ||
stack_root=code_mapping.stacktrace_root, | ||
defaults={ | ||
"repository": repository, | ||
"organization_id": organization.id, | ||
"integration_id": installation.model.id, | ||
"organization_integration_id": installation.org_integration.id, | ||
"source_root": source_path, | ||
"default_branch": branch, | ||
"automatically_generated": True, | ||
"source_root": code_mapping.source_path, | ||
"default_branch": code_mapping.repo.branch, | ||
# This function is called from the UI, thus, we know that the code mapping is user generated | ||
"automatically_generated": False, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a change rather than just a refactor because we have a bunch of user generated code mappings marked as automatically generated (which is not true). |
||
}, | ||
) | ||
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the logic comes from the endpoint: There are some minor changes I indicate below. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import logging | ||
|
||
from rest_framework.request import Request | ||
|
||
from sentry.integrations.source_code_management.repo_trees import ( | ||
RepoAndBranch, | ||
RepoTreesIntegration, | ||
) | ||
from sentry.models.organization import Organization | ||
|
||
from .code_mapping import CodeMapping, CodeMappingTreesHelper, FrameInfo | ||
from .integration_utils import get_installation | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def get_file_and_repo_matches(request: Request, organization: Organization) -> list[dict[str, str]]: | ||
frame_info = get_frame_info_from_request(request) | ||
installation = get_installation(organization) | ||
if not isinstance(installation, RepoTreesIntegration): | ||
return [] | ||
trees = installation.get_trees_for_org() | ||
trees_helper = CodeMappingTreesHelper(trees) | ||
return trees_helper.get_file_and_repo_matches(frame_info) | ||
|
||
|
||
def get_frame_info_from_request(request: Request) -> FrameInfo: | ||
frame = { | ||
"absPath": request.GET.get("absPath"), | ||
# Currently, the only required parameter, thus, avoiding the `get` method | ||
"filename": request.GET["stacktraceFilename"], | ||
"module": request.GET.get("module"), | ||
} | ||
return FrameInfo(frame, request.GET.get("platform")) | ||
|
||
|
||
def get_code_mapping_from_request(request: Request) -> CodeMapping: | ||
return CodeMapping( | ||
repo=RepoAndBranch( | ||
name=request.data["repoName"], | ||
branch=request.data["defaultBranch"], | ||
), | ||
stacktrace_root=request.data["stackRoot"], | ||
source_path=request.data["sourceRoot"], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
from collections.abc import Mapping | ||
|
||
from sentry.integrations.services.integration.model import RpcOrganizationIntegration | ||
from sentry.models.repository import Repository | ||
from sentry.utils import metrics | ||
|
||
from ..constants import METRIC_PREFIX | ||
|
||
|
||
def create_repository( | ||
repo_name: str, org_integration: RpcOrganizationIntegration, tags: Mapping[str, str | bool] | ||
) -> Repository | None: | ||
organization_id = org_integration.organization_id | ||
created = False | ||
repository = ( | ||
Repository.objects.filter(name=repo_name, organization_id=organization_id) | ||
.order_by("-date_added") | ||
.first() | ||
) | ||
if not repository: | ||
if not tags["dry_run"]: | ||
repository, created = Repository.objects.get_or_create( | ||
name=repo_name, | ||
organization_id=organization_id, | ||
integration_id=org_integration.integration_id, | ||
) | ||
if created or tags["dry_run"]: | ||
metrics.incr(key=f"{METRIC_PREFIX}.repository.created", tags=tags, sample_rate=1.0) | ||
|
||
return repository |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import logging | ||
from typing import Literal | ||
|
||
from rest_framework import status | ||
|
@@ -12,17 +13,21 @@ | |
OrganizationIntegrationsLoosePermission, | ||
) | ||
from sentry.api.serializers import serialize | ||
from sentry.issues.auto_source_code_config.code_mapping import ( | ||
create_code_mapping, | ||
derive_code_mappings, | ||
from sentry.issues.auto_source_code_config.code_mapping import NeedsExtension, create_code_mapping | ||
from sentry.issues.auto_source_code_config.derived_code_mappings_endpoint import ( | ||
get_code_mapping_from_request, | ||
get_file_and_repo_matches, | ||
) | ||
from sentry.issues.auto_source_code_config.integration_utils import ( | ||
InstallationCannotGetTreesError, | ||
InstallationNotFoundError, | ||
get_installation, | ||
) | ||
from sentry.models.organization import Organization | ||
from sentry.models.project import Project | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
@region_silo_endpoint | ||
class OrganizationDeriveCodeMappingsEndpoint(OrganizationEndpoint): | ||
|
@@ -33,39 +38,34 @@ class OrganizationDeriveCodeMappingsEndpoint(OrganizationEndpoint): | |
|
||
owner = ApiOwner.ISSUES | ||
publish_status = { | ||
"GET": ApiPublishStatus.UNKNOWN, | ||
"POST": ApiPublishStatus.UNKNOWN, | ||
"GET": ApiPublishStatus.EXPERIMENTAL, | ||
"POST": ApiPublishStatus.EXPERIMENTAL, | ||
} | ||
permission_classes = (OrganizationIntegrationsLoosePermission,) | ||
|
||
def get(self, request: Request, organization: Organization) -> Response: | ||
""" | ||
Get all matches for a stacktrace filename. | ||
Get all files from the customer repositories that match a stack trace frame. | ||
`````````````````` | ||
|
||
:param organization: | ||
:param string absPath: | ||
:param string module: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will need these values for Java calls. |
||
:param string stacktraceFilename: | ||
:param string platform: | ||
:auth: required | ||
""" | ||
stacktrace_filename = request.GET.get("stacktraceFilename") | ||
# XXX: The UI will need to pass the platform | ||
platform = request.GET.get("platform") | ||
|
||
try: | ||
possible_code_mappings = [] | ||
file_repo_matches = [] | ||
resp_status: Literal[200, 204, 400] = status.HTTP_400_BAD_REQUEST | ||
|
||
if stacktrace_filename: | ||
possible_code_mappings = derive_code_mappings( | ||
organization, {"filename": stacktrace_filename}, platform | ||
) | ||
if possible_code_mappings: | ||
resp_status = status.HTTP_200_OK | ||
else: | ||
resp_status = status.HTTP_204_NO_CONTENT | ||
file_repo_matches = get_file_and_repo_matches(request, organization) | ||
if file_repo_matches: | ||
resp_status = status.HTTP_200_OK | ||
else: | ||
resp_status = status.HTTP_204_NO_CONTENT | ||
|
||
return Response(serialize(possible_code_mappings), status=resp_status) | ||
return self.respond(serialize(file_repo_matches), status=resp_status) | ||
except InstallationCannotGetTreesError: | ||
return self.respond( | ||
{"text": "The integration does not support getting trees"}, | ||
|
@@ -76,6 +76,12 @@ def get(self, request: Request, organization: Organization) -> Response: | |
{"text": "Could not find this integration installed on your organization"}, | ||
status=status.HTTP_404_NOT_FOUND, | ||
) | ||
except NeedsExtension: | ||
return self.respond({"text": "Needs extension"}, status=status.HTTP_400_BAD_REQUEST) | ||
except KeyError: | ||
return self.respond( | ||
{"text": "Missing required parameters"}, status=status.HTTP_400_BAD_REQUEST | ||
) | ||
|
||
def post(self, request: Request, organization: Organization) -> Response: | ||
""" | ||
|
@@ -100,19 +106,18 @@ def post(self, request: Request, organization: Organization) -> Response: | |
if not request.access.has_project_access(project): | ||
return self.respond(status=status.HTTP_403_FORBIDDEN) | ||
|
||
repo_name = request.data.get("repoName") | ||
stack_root = request.data.get("stackRoot") | ||
source_root = request.data.get("sourceRoot") | ||
branch = request.data.get("defaultBranch") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All four values are now returned as an object in the call below: |
||
if not repo_name or not stack_root or not source_root or not branch: | ||
try: | ||
installation = get_installation(organization) | ||
# It helps with typing since org_integration can be None | ||
if not installation.org_integration: | ||
raise InstallationNotFoundError | ||
|
||
code_mapping = get_code_mapping_from_request(request) | ||
new_code_mapping = create_code_mapping(organization, code_mapping, project) | ||
except KeyError: | ||
return self.respond( | ||
{"text": "Missing required parameters"}, status=status.HTTP_400_BAD_REQUEST | ||
) | ||
|
||
try: | ||
new_code_mapping = create_code_mapping( | ||
organization, project, stack_root, source_root, repo_name, branch | ||
) | ||
except InstallationNotFoundError: | ||
return self.respond( | ||
{"text": "Could not find this integration installed on your organization"}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,7 @@ def test_get_single_match(self, mock_get_trees_for_org: Any) -> None: | |
} | ||
] | ||
with patch( | ||
"sentry.issues.auto_source_code_config.code_mapping.CodeMappingTreesHelper.list_file_matches", | ||
"sentry.issues.auto_source_code_config.code_mapping.CodeMappingTreesHelper.get_file_and_repo_matches", | ||
return_value=expected_matches, | ||
): | ||
response = self.client.get(self.url, data=config_data, format="json") | ||
|
@@ -73,7 +73,7 @@ def test_get_start_with_backslash(self, mock_get_trees_for_org: Any) -> None: | |
} | ||
] | ||
with patch( | ||
"sentry.issues.auto_source_code_config.code_mapping.CodeMappingTreesHelper.list_file_matches", | ||
"sentry.issues.auto_source_code_config.code_mapping.CodeMappingTreesHelper.get_file_and_repo_matches", | ||
return_value=expected_matches, | ||
): | ||
response = self.client.get(self.url, data=config_data, format="json") | ||
|
@@ -103,7 +103,7 @@ def test_get_multiple_matches(self, mock_get_trees_for_org: Any) -> None: | |
}, | ||
] | ||
with patch( | ||
"sentry.issues.auto_source_code_config.code_mapping.CodeMappingTreesHelper.list_file_matches", | ||
"sentry.issues.auto_source_code_config.code_mapping.CodeMappingTreesHelper.get_file_and_repo_matches", | ||
return_value=expected_matches, | ||
): | ||
response = self.client.get(self.url, data=config_data, format="json") | ||
|
@@ -156,7 +156,7 @@ def test_post_simple(self) -> None: | |
repo = Repository.objects.get(name="getsentry/codemap") | ||
assert response.status_code == 201, response.content | ||
assert response.data == { | ||
"automaticallyGenerated": True, | ||
"automaticallyGenerated": False, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the behaviour change I mentioned earlier. |
||
"id": str(response.data["id"]), | ||
"projectId": str(self.project.id), | ||
"projectSlug": self.project.slug, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
CodeMapping
type contains all four variables I'm replacing it with.