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

Improve OpenAPI schema coverage #3629

Merged
merged 44 commits into from
Jan 12, 2024
Merged

Improve OpenAPI schema coverage #3629

merged 44 commits into from
Jan 12, 2024

Conversation

vadimkerr
Copy link
Member

@vadimkerr vadimkerr commented Jan 8, 2024

What this PR does

Improves OpenAPI schema coverage for internal API:

  • Fixes/Improves alert group and feature endpoints
  • Adds integration and user endpoints

Which issue(s) this PR fixes

#3444

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

@vadimkerr vadimkerr added pr:no public docs Added to a PR that does not require public documentation updates pr:no changelog labels Jan 9, 2024
@@ -54,101 +58,62 @@ def get_user_queryset(request):
return User.objects.filter(organization=request.user.organization).distinct()


class AlertGroupFilterBackend(filters.DjangoFilterBackend):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this as it doesn't work (the number of SQL queries is the same without this code)

class AlertGroupFilter(DateRangeFilterMixin, ModelFieldFilterMixin, filters.FilterSet):
"""
Examples of possible date formats here https://docs.djangoproject.com/en/1.9/ref/settings/#datetime-input-formats
"""

FILTER_BY_INVOLVED_USERS_ALERT_GROUPS_CUTOFF = 1000

started_at_gte = filters.DateTimeFilter(field_name="started_at", lookup_expr="gte")
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing outdated fields:

  • started_at_gte
  • started_at_lte
  • resolved_at_gte
  • resolved_at_lte
  • id__in

Comment on lines -140 to -152
class Meta:
model = AlertGroup
fields = [
"id__in",
"started_at_gte",
"started_at_lte",
"resolved_at_lte",
"is_root",
"resolved_by",
"acknowledged_by",
]

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 not needed, the filters work the same way without this

@@ -305,15 +268,12 @@ class AlertGroupView(
"preview_template": [RBACPermission.Permissions.INTEGRATIONS_TEST],
}

http_method_names = ["get", "post", "delete"]
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 outdated

@action(methods=["get"], detail=False)
def filters(self, request):
"""
Retrieve a list of valid filter options that can be used to filter alert groups
"""
filter_name = request.query_params.get("search", None)
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 outdated/not used by API clients

@action(methods=["get"], detail=False)
def filters(self, request):
organization = self.request.auth.organization
filter_name = request.query_params.get("search", None)
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 outdated/not used by API clients

Copy link
Member Author

Choose a reason for hiding this comment

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

simplifying paginators + adding schema for them

@vadimkerr vadimkerr changed the title OpenAPI improvements Improve OpenAPI schema coverage Jan 10, 2024
@vadimkerr vadimkerr marked this pull request as ready for review January 10, 2024 19:59
@vadimkerr vadimkerr requested a review from a team January 10, 2024 19:59
Copy link
Contributor

@mderynck mderynck left a comment

Choose a reason for hiding this comment

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

LGTM, could probably use another quick pass by someone else given the size 🙂

@vadimkerr vadimkerr added this pull request to the merge queue Jan 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 12, 2024
@vadimkerr vadimkerr merged commit d0904ca into dev Jan 12, 2024
21 checks passed
@vadimkerr vadimkerr deleted the vadimkerr/openapi-improvements branch January 12, 2024 15:11
iskhakov pushed a commit that referenced this pull request Feb 20, 2024
Improves OpenAPI schema coverage for internal API:

- Fixes/Improves `alert group` and `feature` endpoints
- Adds `integration` and `user` endpoints

#3444

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants