Skip to content

feat(events): Use SnubaEvent if option is turned on #12594

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

Merged
merged 3 commits into from
Apr 15, 2019

Conversation

lynnagara
Copy link
Member

@lynnagara lynnagara commented Mar 29, 2019

Prevents 404s on various events endpoints caused by sampling events when Snuba is on

Prevents 404s on event owners caused by sampling events when Snuba is on
@lynnagara
Copy link
Member Author

We probably have to do the same in a few more endpoints, I wonder if there's any better solution

@lynnagara lynnagara changed the title feat(event-owners): Use SnubaEvent if option is turned on feat(events): Use SnubaEvent if option is turned on Mar 29, 2019

event_cls = SnubaEvent if use_snuba else Event

event = event_cls.objects.from_event_id(event_id, project.id)
Copy link
Member

Choose a reason for hiding this comment

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

Would it be outlandish to make Event.objects.from_event_id do this logic internally? That might help you play fewer rounds of whac-a-mole.

@lynnagara
Copy link
Member Author

Think I hit all the ones we need to update now

@lynnagara lynnagara requested a review from JTCunning April 8, 2019 18:04
@JTCunning JTCunning removed their request for review April 8, 2019 18:11
@JTCunning
Copy link
Contributor

Removing myself from review, but giving a generic +1 and following along.

Copy link
Contributor

@alex-hofsteede alex-hofsteede left a comment

Choose a reason for hiding this comment

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

Yeah this seems good.

+0.5 to Mark's comment about having the EventManager logic do this SnubaEvent/Event switching internally. If it can be done that way easily without coupling the 2 event types too much, then I would be for it.

@lynnagara lynnagara merged commit 2242c01 into master Apr 15, 2019
@lynnagara lynnagara deleted the feat/event-owners-snubaevent branch April 15, 2019 18:31
jan-auer added a commit that referenced this pull request Apr 16, 2019
* master: (50 commits)
  fix(ui) Don't show save-org-search on event search (#12785)
  ref(ui): Remove some unnecessary index.jsx files (#12606)
  feat(app-platform): Analytics (#12718)
  ref(js): Remove ApiMixin (#12384)
  test(js): Silence project plugin console info spam (#12761)
  test(js): Move SaveSearchStore.reset() (#12769)
  test(js): Add more fields to Group fixture (#12759)
  feat(app-platform): Integration "Learn More" modal (#12638)
  feat(saved-searches) Move create saved search button to search bar. (#12781)
  ref(global-header): Remove dead code (#12767)
  ref(releases): Refactored Releases Serializers (#12535)
  feat(app-platform): Sort Integrations (#12697)
  ref(audit-log): Log sso config updates (#12744)
  ref(app-platform): New 'Open In' UI  (#12621)
  feat(events): Use SnubaEvent if option is turned on (#12594)
  feat(global-selection-header): show settings icon link in single project mode (#12772)
  refs(api): Consolidate all search backend code into `SnubaSearchBackend`
  fix(tests) Remove large snapshots (#12766)
  fix: Update symbolicator snapshots (#12710)
  ref: Upgrade semaphore (#12751)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to 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