-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(prevent): create endpoint that fetches repositories list #95181
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
base: master
Are you sure you want to change the base?
Changes from all commits
965b8ad
1bb2065
0ef00c4
20a7b9f
36cebd2
1faba51
d32e19e
d3bf201
c8dbbd1
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 |
---|---|---|
@@ -0,0 +1,38 @@ | ||
query = """query ReposForOwner( | ||
$owner: String! | ||
$filters: RepositorySetFilters! | ||
$ordering: RepositoryOrdering! | ||
$direction: OrderingDirection! | ||
$first: Int | ||
$after: String | ||
$last: Int | ||
$before: String | ||
) { | ||
owner(username: $owner) { | ||
repositories( | ||
filters: $filters | ||
ordering: $ordering | ||
orderingDirection: $direction | ||
first: $first | ||
after: $after | ||
last: $last | ||
before: $before | ||
) { | ||
edges { | ||
node { | ||
adrianviquez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name | ||
updatedAt | ||
latestCommitAt | ||
defaultBranch | ||
} | ||
} | ||
pageInfo { | ||
startCursor | ||
endCursor | ||
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. Can you also add hasPreviousPage and startCursor? |
||
hasNextPage | ||
hasPreviousPage | ||
} | ||
totalCount | ||
} | ||
} | ||
}""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
from drf_spectacular.utils import extend_schema | ||
from rest_framework import status | ||
from rest_framework.request import Request | ||
from rest_framework.response import Response | ||
|
||
from sentry.api.api_owners import ApiOwner | ||
from sentry.api.api_publish_status import ApiPublishStatus | ||
from sentry.api.base import region_silo_endpoint | ||
from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND | ||
from sentry.apidocs.parameters import GlobalParams, PreventParams | ||
from sentry.codecov.base import CodecovEndpoint | ||
from sentry.codecov.client import CodecovApiClient | ||
from sentry.codecov.endpoints.Repositories.query import query | ||
from sentry.codecov.endpoints.Repositories.serializers import RepositoriesSerializer | ||
from sentry.codecov.enums import OrderingDirection | ||
|
||
MAX_RESULTS_PER_PAGE = 50 | ||
|
||
|
||
@extend_schema(tags=["Prevent"]) | ||
@region_silo_endpoint | ||
class RepositoriesEndpoint(CodecovEndpoint): | ||
__test__ = False | ||
|
||
owner = ApiOwner.CODECOV | ||
publish_status = { | ||
"GET": ApiPublishStatus.PUBLIC, | ||
} | ||
|
||
@extend_schema( | ||
operation_id="Retrieves repository list for a given owner", | ||
parameters=[ | ||
GlobalParams.ORG_ID_OR_SLUG, | ||
PreventParams.OWNER, | ||
PreventParams.FIRST, | ||
PreventParams.LAST, | ||
PreventParams.CURSOR, | ||
PreventParams.TERM, | ||
], | ||
request=None, | ||
responses={ | ||
200: RepositoriesSerializer, | ||
400: RESPONSE_BAD_REQUEST, | ||
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. If you're up for it, one thing I didn't do with my initial implementation of the serializers with the test results one is to actually throw the exceptions in the serializer if we error out on the owner part of the request. https://github.com/codecov/umbrella/blob/02de63009831bee26cb65113b35f6ef53ed32a3b/apps/codecov-api/graphql_api/types/repository/repository.graphql#L95 All of this stuff basically would be in the GQL request IIRC. But for repositories also maybe not, because it returns a set of RepositoryConnections, in which case idk when we'd return 404 or 403 🤔 Something we can maybe talk about later |
||
403: RESPONSE_FORBIDDEN, | ||
404: RESPONSE_NOT_FOUND, | ||
}, | ||
) | ||
def get(self, request: Request, owner: str, **kwargs) -> Response: | ||
""" | ||
Retrieves repository data for a given owner. | ||
""" | ||
|
||
first_param = request.query_params.get("first") | ||
last_param = request.query_params.get("last") | ||
cursor = request.query_params.get("cursor") | ||
|
||
# When calling request.query_params, the URL is decoded so + is replaced with spaces. We need to change them back so Codecov can properly fetch the next page. | ||
if cursor: | ||
cursor = cursor.replace(" ", "+") | ||
|
||
try: | ||
first = int(first_param) if first_param is not None else None | ||
last = int(last_param) if last_param is not None else None | ||
except ValueError: | ||
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. can we add some simple tests for these error responses? |
||
return Response( | ||
status=status.HTTP_400_BAD_REQUEST, | ||
data={"details": "Query parameters 'first' and 'last' must be integers."}, | ||
) | ||
|
||
if first is not None and last is not None: | ||
return Response( | ||
status=status.HTTP_400_BAD_REQUEST, | ||
data={"details": "Cannot specify both `first` and `last`"}, | ||
) | ||
|
||
if first is None and last is None: | ||
first = MAX_RESULTS_PER_PAGE | ||
|
||
variables = { | ||
"owner": owner, | ||
"filters": {"term": request.query_params.get("term")}, | ||
"direction": OrderingDirection.DESC.value, | ||
adrianviquez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"ordering": "COMMIT_DATE", | ||
"first": first, | ||
"last": last, | ||
"before": cursor if cursor and last else None, | ||
"after": cursor if cursor and first else None, | ||
} | ||
|
||
client = CodecovApiClient(git_provider_org=owner) | ||
graphql_response = client.query(query=query, variables=variables) | ||
repositories = RepositoriesSerializer().to_representation(graphql_response.json()) | ||
|
||
return Response(repositories) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import logging | ||
|
||
import sentry_sdk | ||
from rest_framework import serializers | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class RepositoryNodeSerializer(serializers.Serializer): | ||
""" | ||
Serializer for individual repository nodes from GraphQL response | ||
""" | ||
|
||
__test__ = False | ||
|
||
name = serializers.CharField() | ||
adrianviquez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
updatedAt = serializers.DateTimeField() | ||
latestCommitAt = serializers.DateTimeField() | ||
defaultBranch = serializers.CharField() | ||
|
||
|
||
class PageInfoSerializer(serializers.Serializer): | ||
""" | ||
Serializer for pagination information | ||
""" | ||
|
||
startCursor = serializers.CharField(allow_null=True) | ||
endCursor = serializers.CharField(allow_null=True) | ||
hasNextPage = serializers.BooleanField() | ||
hasPreviousPage = serializers.BooleanField() | ||
|
||
|
||
class RepositoriesSerializer(serializers.Serializer): | ||
""" | ||
Serializer for repositories response | ||
""" | ||
|
||
__test__ = False | ||
|
||
results = RepositoryNodeSerializer(many=True) | ||
pageInfo = PageInfoSerializer() | ||
totalCount = serializers.IntegerField() | ||
|
||
def to_representation(self, graphql_response): | ||
""" | ||
Transform the GraphQL response to the serialized format | ||
""" | ||
try: | ||
repository_data = graphql_response["data"]["owner"]["repositories"] | ||
repositories = repository_data["edges"] | ||
page_info = repository_data.get("pageInfo", {}) | ||
|
||
nodes = [] | ||
for edge in repositories: | ||
node = edge["node"] | ||
nodes.append(node) | ||
|
||
response_data = { | ||
"results": nodes, | ||
"pageInfo": repository_data.get( | ||
"pageInfo", | ||
{ | ||
"hasNextPage": page_info.get("hasNextPage"), | ||
"hasPreviousPage": page_info.get("hasPreviousPage"), | ||
"startCursor": page_info.get("startCursor"), | ||
"endCursor": page_info.get("endCursor"), | ||
}, | ||
), | ||
"totalCount": repository_data.get("totalCount", len(nodes)), | ||
} | ||
|
||
return super().to_representation(response_data) | ||
|
||
except (KeyError, TypeError) as e: | ||
sentry_sdk.capture_exception(e) | ||
logger.exception( | ||
"Error parsing GraphQL response", | ||
extra={ | ||
"error": str(e), | ||
"endpoint": "repositories", | ||
"response_keys": ( | ||
list(graphql_response.keys()) | ||
if isinstance(graphql_response, dict) | ||
else None | ||
), | ||
}, | ||
) | ||
raise |
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.
man, this made me realize I probably should've named the other routes repositories rather than repository, maybe I can do a follow up on this