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.

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

ref(derived_code_mappings): Refactor endpoint #84318

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Jan 30, 2025

This also makes the code less coupled to the GitHub integration.

This also makes the code less coupled to the GitHub integration.
@armenzg armenzg self-assigned this Jan 30, 2025
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 30, 2025
@@ -286,29 +285,6 @@ def has_repo_access(self, repo: RpcRepository) -> bool:
return False
return True

# XXX: To be removed after we switch over to the new repo trees integration code
def get_trees_for_org_old(self, cache_seconds: int = 3600 * 24) -> dict[str, RepoTree]:
Copy link
Member Author

Choose a reason for hiding this comment

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

I should have removed this in #84164

@@ -45,6 +48,23 @@ class UnsupportedFrameFilename(Exception):
pass


def derive_code_mappings(
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the code the endpoint is going to call.

Eventually, I will unify this code and the one the task calls.

src/sentry/issues/auto_source_code_config/constants.py Outdated Show resolved Hide resolved
SUPPORTED_INTEGRATIONS = ["github"]


def get_installation(
Copy link
Member Author

Choose a reason for hiding this comment

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

This function comes from the task module but it needs to be shared.

@@ -71,6 +73,9 @@ def process_event(project_id: int, group_id: int, event_id: str) -> None:
logger.info("No installation or organization integration found.", extra=extra)
return

if not isinstance(installation, RepoTreesIntegration):
Copy link
Member Author

Choose a reason for hiding this comment

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

This class is what gives an integration the get_trees_for_org method.

@@ -184,30 +189,6 @@ def get_stacktrace(data: NodeData) -> list[Mapping[str, Any]]:
return []


def get_installation(
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to a shared module.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm moving code from here into the code_mapping module.

resp_status: Literal[200, 204, 400] = status.HTTP_400_BAD_REQUEST

if stacktrace_filename:
possible_code_mappings = derive_code_mappings(organization, stacktrace_filename)
Copy link
Member Author

Choose a reason for hiding this comment

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

All code mapping-related code has been moved within this call.

Copy link

codecov bot commented Jan 30, 2025

❌ 27 Tests Failed:

Tests completed Failed Passed Skipped
23691 27 23664 294
View the top 3 failed tests by shortest run time
tests.sentry.issues.auto_source_code_config.test_task.TestGoDeriveCodeMappings::test_auto_source_code_config_go_long_abs_filename
Stack Traces | 2.41s run time
#x1B[1m#x1B[.../issues/auto_source_code_config/test_task.py#x1B[0m:451: in test_auto_source_code_config_go_long_abs_filename
    code_mapping = RepositoryProjectPathConfig.objects.all()[0]
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:452: in __getitem__
    return qs._result_cache[0]
#x1B[1m#x1B[31mE   IndexError: list index out of range#x1B[0m
tests.sentry.issues.auto_source_code_config.test_task.TestTaskBehavior::test_does_not_raise_installation_removed_old_code_path
Stack Traces | 2.42s run time
#x1B[1m#x1B[.../issues/auto_source_code_config/test_task.py#x1B[0m:75: in test_does_not_raise_installation_removed_old_code_path
    assert_halt_metric(mock_record, error)
#x1B[1m#x1B[.../sentry/testutils/asserts.py#x1B[0m:64: in assert_halt_metric
    (event_halts,) = (
#x1B[1m#x1B[31mE   ValueError: not enough values to unpack (expected 1, got 0)#x1B[0m
tests.sentry.issues.auto_source_code_config.test_task.TestBackSlashDeriveCodeMappings::test_backslash_drive_letter_filename_simple
Stack Traces | 2.45s run time
#x1B[1m#x1B[.../issues/auto_source_code_config/test_task.py#x1B[0m:132: in test_backslash_drive_letter_filename_simple
    code_mapping = RepositoryProjectPathConfig.objects.all()[0]
#x1B[1m#x1B[31m.venv/lib/python3.13.../db/models/query.py#x1B[0m:452: in __getitem__
    return qs._result_cache[0]
#x1B[1m#x1B[31mE   IndexError: list index out of range#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant