Skip to content

Conversation

@rbro112
Copy link
Member

@rbro112 rbro112 commented Oct 24, 2025

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Copy link
Member Author

rbro112 commented Oct 24, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

organization_id = project.organization_id
project_id = project.id

# object_key = f"{organization_id}/{project_id}/{app_icon_id}"
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 think we'll want to preserve this path

@rbro112 rbro112 changed the title Add app-icon download endpoint feat(preprod): Add app-icon download endpoint Oct 28, 2025
@rbro112 rbro112 marked this pull request as ready for review October 28, 2025 19:35
@rbro112 rbro112 requested a review from a team as a code owner October 28, 2025 19:35
Comment on lines +5 to +6
from sentry.preprod.api.endpoints.project_preprod_artifact_icon import (
ProjectPreprodArtifactImageEndpoint,
Copy link

Choose a reason for hiding this comment

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

Bug: urls.py imports ProjectPreprodArtifactImageEndpoint from a non-existent module project_preprod_artifact_icon.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The urls.py file attempts to import ProjectPreprodArtifactImageEndpoint from sentry.preprod.api.endpoints.project_preprod_artifact_icon on line 5. However, the module project_preprod_artifact_icon.py does not exist. The intended class ProjectPreprodArtifactImageEndpoint is defined in project_preprod_artifact_image.py. This mismatch will cause a ModuleNotFoundError when the application attempts to load URL patterns, preventing the application from starting.

💡 Suggested Fix

Correct the import path in src/sentry/preprod/api/endpoints/urls.py from sentry.preprod.api.endpoints.project_preprod_artifact_icon to sentry.preprod.api.endpoints.project_preprod_artifact_image.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/preprod/api/endpoints/urls.py#L5-L6

Potential issue: The `urls.py` file attempts to import
`ProjectPreprodArtifactImageEndpoint` from
`sentry.preprod.api.endpoints.project_preprod_artifact_icon` on line 5. However, the
module `project_preprod_artifact_icon.py` does not exist. The intended class
`ProjectPreprodArtifactImageEndpoint` is defined in `project_preprod_artifact_image.py`.
This mismatch will cause a `ModuleNotFoundError` when the application attempts to load
URL patterns, preventing the application from starting.

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

)

# Upload failed, return appropriate error
return HttpResponse({"error": "Not found"}, status=404)
Copy link

Choose a reason for hiding this comment

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

Bug: HttpResponse is incorrectly used with dictionary objects at lines 117, 129, 140.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The code at lines 117, 129, and 140 attempts to return HttpResponse with Python dictionary objects, such as HttpResponse({"error": "Not found"}, status=404). The HttpResponse constructor does not accept dictionaries directly; it expects content as strings, bytes, or iterators. This will result in a TypeError during response serialization, causing a generic 500 error to be returned to the client instead of the intended structured error response.

💡 Suggested Fix

Replace HttpResponse with JsonResponse when returning dictionary objects. For example, change return HttpResponse({"error": "Not found"}, status=404) to return JsonResponse({"error": "Not found"}, status=404).

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/preprod/api/endpoints/project_preprod_artifact_image.py#L117

Potential issue: The code at lines 117, 129, and 140 attempts to return `HttpResponse`
with Python dictionary objects, such as `HttpResponse({"error": "Not found"},
status=404)`. The `HttpResponse` constructor does not accept dictionaries directly; it
expects content as strings, bytes, or iterators. This will result in a `TypeError`
during response serialization, causing a generic 500 error to be returned to the client
instead of the intended structured error response.

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

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.

3 participants