ref: Remove unused Group.get_oldest_event and legacy events behavior #14038

lynnagara · 2019-07-16T18:38:56Z

Removes the "snuba.events-queries.enabled" option and legacy code paths
that were not being used in production. Also removes the unused
Group.get_oldest_event method. Part 4 of #13905

fpacifici

There are a couple of things to fix in the tests.

fpacifici · 2019-07-17T00:13:13Z

tests/sentry/models/test_group.py

+        self.min_ago = (timezone.now() - timedelta(minutes=1)).isoformat()[:19]
+        self.two_min_ago = (timezone.now() - timedelta(minutes=2)).isoformat()[:19]
+        self.sec_ago = (timezone.now() - timedelta(seconds=1)).isoformat()[:19]
+        self.two_sec_ago = (timezone.now() - timedelta(seconds=1)).isoformat()[:19]


Unless you are moving close to the speed of light, thus introducing relativistic effects like time dilation, this is still one sec ago and not two sec ago.
Technically we do not know where travis runs CI so we cannot exclude the relativistic option.

fpacifici · 2019-07-17T00:25:47Z

tests/sentry/models/test_group.py

-                event_id=six.text_type(i),
-                group=group,
-                datetime=datetime(2013, 8, 13, 3, 8, 30),  # all in the middle
+    def test_get_latest_identical_timestamps(self):


Is this an invariant we want to test for ?
I am not sure the product should depend on the fact that 3 events carrying the same timestamp will be returned in the order in which they are stored.

Probably doesn't make much sense anymore, will remove this test

fpacifici · 2019-07-17T00:27:46Z

tests/sentry/models/test_group.py

+            data={
+                'event_id': 'a' * 32,
+                'fingerprint': ['group-1'],
+                'timestamp': self.two_sec_ago,


I'd turn these into 60 seconds ago and 61 seconds ago to avoid test flakiness.
When you query snuba, it rounds the last timestamp to one second ago, which means you may miss the event added at line 95 one in a while.

Removes the "snuba.events-queries.enabled" option and legacy code paths that were not being used in production. Also removes the unused Group.get_oldest_event method. Part 4 of #13905

lynnagara · 2019-07-17T23:17:15Z

Updated to base on master

fpacifici · 2019-07-18T00:54:37Z

tests/sentry/models/test_group.py

+            data={
+                'event_id': 'b' * 32,
+                'fingerprint': ['group-1'],
+                'timestamp': self.sec_ago,


These are not almost identical anymore

Oops, thanks

codecov · 2019-07-18T18:02:16Z

Codecov Report

Merging #14038 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #14038      +/-   ##
==========================================
- Coverage   86.81%   86.81%   -0.01%     
==========================================
  Files        3222     3222              
  Lines      140420   140400      -20     
  Branches     4952     4952              
==========================================
- Hits       121906   121887      -19     
+ Misses      17124    17123       -1     
  Partials     1390     1390

Impacted Files	Coverage Δ
tests/sentry/models/test_group.py	`100% <100%> (ø)`	⬆️
src/sentry/models/group.py	`92.71% <100%> (+0.34%)`	⬆️
src/sentry/utils/metrics.py	`84.61% <0%> (-3.3%)`	⬇️
src/sentry/models/environment.py	`98.48% <0%> (+3.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d320b1...0f671c7. Read the comment docs.

fpacifici · 2019-07-18T20:41:14Z

tests/sentry/models/test_group.py

+        super(GroupTest, self).setUp()
+        self.min_ago = (timezone.now() - timedelta(minutes=1)).isoformat()[:19]
+        self.two_min_ago = (timezone.now() - timedelta(minutes=2)).isoformat()[:19]
+        self.sec_ago = (timezone.now() - timedelta(seconds=1)).isoformat()[:19]


Not used. You can remove it

* master: ref(admin): Convert user edit page to react (#14074) ref: Remove unused Group.get_oldest_event and legacy events behavior (#14038) ref(api): Update DELETE users/ to support hard deleting (#14068) test(OrganizationDiscoverSavedQueryDetailTest): Stabilize put test (#14077) meta(readme): Sentry logo should link to sentry.io (#14076) ref: Remove duplicate column (#14073) App platform/update permissions token auth (#14046) feat: Support issue IDs as canonical parameters ref: Change to new traceparent header for Python SDK (#14070) feat: Use option to force-disable transaction events (#14056) feat(apm): Register option to force-disable transaction events (#14055) Feat/mark sentry app installed put route (#14060) ref: Remove unused Group.event_set property (#14036) fix: Filter out groups that are pending deletion/merge from `by_qualified_short_id` (SEN-849) fix(ui): Fix resolve/ignore actions for accounts without multi… (#14058) Fix: Remove extra $.param introduced in GH-14051 (#14061) feat: Use Snuba for Group.from_event_id (#14034) fix(ui) Display implicit default sort and default to descending (#14042) fix(github) Fix 404s not being handled in repository search (#14030) fix: Pass an empty array to $.param instead of an empty string when options.query is falsey (#14051) # Conflicts: # src/sentry/utils/sdk.py

lynnagara mentioned this pull request Jul 16, 2019

feat: Use Snuba events in group model #13905

Closed

lynnagara changed the base branch from master to feat/group-model-3 July 16, 2019 18:58

lynnagara requested a review from a team July 17, 2019 00:08

fpacifici requested changes Jul 17, 2019

View reviewed changes

lynnagara force-pushed the feat/group-model-3 branch from f53a9c9 to 7af0c0f Compare July 17, 2019 18:06

lynnagara changed the base branch from feat/group-model-3 to master July 17, 2019 23:08

lynnagara added 2 commits July 17, 2019 16:16

ref: Remove unused Group.get_oldest_event and legacy events behavior

9475051

Removes the "snuba.events-queries.enabled" option and legacy code paths that were not being used in production. Also removes the unused Group.get_oldest_event method. Part 4 of #13905

fix test timestamps

fd937c7

lynnagara force-pushed the group-model-4 branch from ee4d946 to fd937c7 Compare July 17, 2019 23:16

lynnagara mentioned this pull request Jul 17, 2019

feat: Update Group.get_latest_event to use Snuba event #14039

Merged

fpacifici requested changes Jul 18, 2019

View reviewed changes

fix almost identical test

0f671c7

fpacifici reviewed Jul 18, 2019

View reviewed changes

fpacifici approved these changes Jul 18, 2019

View reviewed changes

remove unused value

a73f5ce

lynnagara merged commit 9e66ffc into master Jul 18, 2019

evanpurkhiser deleted the group-model-4 branch July 24, 2019 00:52

github-actions bot locked and limited conversation to collaborators Dec 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ref: Remove unused Group.get_oldest_event and legacy events behavior #14038

ref: Remove unused Group.get_oldest_event and legacy events behavior #14038

Uh oh!

lynnagara commented Jul 16, 2019

Uh oh!

fpacifici left a comment

Uh oh!

fpacifici Jul 17, 2019

Uh oh!

fpacifici Jul 17, 2019

Uh oh!

lynnagara Jul 17, 2019

Uh oh!

fpacifici Jul 17, 2019

Uh oh!

lynnagara commented Jul 17, 2019

Uh oh!

fpacifici Jul 18, 2019

Uh oh!

lynnagara Jul 18, 2019

Uh oh!

codecov bot commented Jul 18, 2019

Uh oh!

fpacifici Jul 18, 2019

Uh oh!

Uh oh!

Uh oh!

ref: Remove unused Group.get_oldest_event and legacy events behavior #14038

ref: Remove unused Group.get_oldest_event and legacy events behavior #14038

Uh oh!

Conversation

lynnagara commented Jul 16, 2019

Uh oh!

fpacifici left a comment

Choose a reason for hiding this comment

Uh oh!

fpacifici Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

fpacifici Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

lynnagara Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

fpacifici Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

lynnagara commented Jul 17, 2019

Uh oh!

fpacifici Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

lynnagara Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 18, 2019

Codecov Report

Uh oh!

fpacifici Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!