Skip to content

feat(discover): Add docs for organization_events - #34768

Merged
wmak merged 11 commits into
masterfrom
wmak/feat/org-events-docs
Jun 2, 2022
Merged

feat(discover): Add docs for organization_events#34768
wmak merged 11 commits into
masterfrom
wmak/feat/org-events-docs

Conversation

@wmak

@wmakwmak commented May 18, 2022

Copy link
Copy Markdown
Member
  • This adds a new doc sections for Visibility called Discover and Performance
  • This adds the first Discover endpoint we're publicizing the events endpoint (which replaces the soon to be deprecated eventsv2 endpoint) as the first endpoint in the documentation

Preview

image

- This adds a new doc sections for Visibility called `Discover and
Performance`
type=str,
location="path",
)
STATS_PERIOD = OpenApiParameter(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think OpenApiParameter maps to URL parameters, not query parameters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

drf serializers map to query parameters

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so, its what the location param on the OpenApiParameter is for so you can define whether the parameter is a part of the path or the query

Based on docs you can do header, cookie or form parameters too:
https://drf-spectacular.readthedocs.io/en/latest/drf_yasg.html?highlight=location#parameter-location



class VISIBILITY_PARAMS:
QUERY = OpenApiParameter(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this a visibility-only param? I think we use it in several places outside of visibility.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This is a specific Query to Visibility (see the bit about the search syntax)

@wmak
wmak requested a review from AniketDas-TekkyJune 1, 2022 15:47

@edwardgou-sentryedwardgou-sentry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have some suggestions! Feel free to pick and choose or edit.

Comment threadsrc/sentry/api/endpoints/organization_events.py Outdated
Comment threadsrc/sentry/api/endpoints/organization_events.py Outdated
Comment threadsrc/sentry/api/endpoints/organization_events.py Outdated
Comment threadsrc/sentry/api/endpoints/organization_events.py Outdated
Comment threadsrc/sentry/api/endpoints/organization_events.py Outdated
Comment threadsrc/sentry/apidocs/parameters.py Outdated
Comment threadsrc/sentry/apidocs/parameters.py Outdated
Comment threadsrc/sentry/apidocs/parameters.py Outdated
Comment threadsrc/sentry/apidocs/parameters.py Outdated
Comment threadsrc/sentry/apidocs/parameters.py Outdated
@edwardgou-sentry

Copy link
Copy Markdown
Contributor

Also, probably out of scope for this pr, but I think this part should say something like, You need authorization via bearer auth token, since authentication is a different thing. This part lives in the docs repo though so definitely not for this pr.

image

Comment threadsrc/sentry/apidocs/parameters.py Outdated
Co-authored-by: edwardgou-sentry <83961295+edwardgou-sentry@users.noreply.github.com>
Co-authored-by: Ash Anand <0Calories@users.noreply.github.com>
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@wmak@edwardgou-sentry@0Calories@AniketDas-Tekky