-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(api): Support Project ID and Slugs in OrganizationArtifactBundleAssembleEndpoint
#74232
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #74232 +/- ##
=======================================
Coverage 78.12% 78.13%
=======================================
Files 6665 6665
Lines 297919 297927 +8
Branches 51280 51282 +2
=======================================
+ Hits 232764 232772 +8
- Misses 58878 58882 +4
+ Partials 6277 6273 -4
|
src/sentry/api/endpoints/organization_artifactbundle_assemble.py
Outdated
Show resolved
Hide resolved
src/sentry/api/endpoints/organization_artifactbundle_assemble.py
Outdated
Show resolved
Hide resolved
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.
some nits but overall lgtm!
src/sentry/api/endpoints/organization_artifactbundle_assemble.py
Outdated
Show resolved
Hide resolved
@@ -141,6 +141,86 @@ def test_assemble_with_invalid_projects(self): | |||
assert response.status_code == 400, response.content | |||
assert response.data["error"] == "One or more projects are invalid" | |||
|
|||
def test_assemble_with_valid_project_slugs(self): |
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.
subjective nit: maybe this test isn't needed b/c we're using project slug already for everything else? This is prob more subjective tho cuz maybe it's good to explicitly test anyways. Up to you
9ef3636
to
b1da53b
Compare
I am working on getting all the commands in our Sentry CLI to support Ids as well as Slugs. rough spec
We have already gone through the effort of supporting ids and slugs in path parameters of all the APIs in our codebase. There are some endpoints that the CLI uses that pass in project slugs as body parameters, so we need to support Ids for those as well.
Here, I use the fact that ids are numeric and slugs aren't to process both type of identifiers.