-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
featu(search) Use snuba to fetch events by default #13700
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
mattrobenolt
left a comment
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.
Why not just remove the switch instead? If it’s defaulted to true, there’s 0 reason for someone to turn it off.
mitsuhiko
left a comment
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.
lgtm but i would indeed remove all the now dead code that hangs on this switch.
|
I cannot remove the switch yet because there is a handful of tests that force it to False. I will address them in a separate PR. |
24acdb4 to
5d6fb4d
Compare
| min_ago = (timezone.now() - timedelta(minutes=1)).isoformat()[:19] | ||
| self.event = self.store_event( | ||
| data={ | ||
| # 'event_id': 'a' * 32, |
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.
You could remove this 🚲
| # 'event_id': 'a' * 32, | ||
| 'fingerprint': ['group1'], | ||
| 'timestamp': min_ago, | ||
| # 'tags': {'sentry:release': release.version}, |
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.
More commented code that could be removed.
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.
weird, I thought I removed all of them. Will fix
snuba.events-queries.enabled option is already active in production. This commit activates it by default and fixes the tests that did not support it. This step is needed in order to remove our dependency on PG events which is useful for issueless events.
0643a49 to
cbabee5
Compare
#13700 changed the way test events are generated. The result is that, in acceptance tests, the time the event is created is not normalized to the day thus the screenshot of the details page is different at every execution breaking the snapshot tests on percy. This normalizes the creation timestamp thus should make the snapshot consistent.
snuba.events-queries.enabled option is supposed to be already active in production. This commit activates it by default and fixes the tests that did not support it.
This step is needed in order to remove our dependency on PG events which is useful for issueless
events.