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

fix(recommended-event): Use event ID as tie breaker to match latest event #60323

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

malwilley
Copy link
Member

@malwilley malwilley commented Nov 20, 2023

Sometimes the recommended event would not match the latest event despite everything about the event being the same. I believe this is because the latest event logic uses event_id in the sort, but recommended does not:

LATEST = ["-timestamp", "-event_id"]

Adds event_id as the last item in the sort to break ties when the timestamp and everything else is the same.

@malwilley malwilley requested a review from a team November 20, 2023 22:25
@malwilley malwilley requested a review from a team as a code owner November 20, 2023 22:25
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 20, 2023
@malwilley malwilley removed the request for review from a team November 20, 2023 22:26
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #60323 (a3a743c) into master (0f418cb) will increase coverage by 0.00%.
Report is 7 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #60323   +/-   ##
=======================================
  Coverage   80.85%   80.85%           
=======================================
  Files        5183     5183           
  Lines      227698   227698           
  Branches    38316    38316           
=======================================
+ Hits       184110   184114    +4     
+ Misses      37965    37961    -4     
  Partials     5623     5623           
Files Coverage Δ
src/sentry/models/group.py 94.49% <ø> (ø)

... and 6 files with indirect coverage changes

@malwilley malwilley merged commit 8a58ecb into master Nov 21, 2023
52 checks passed
@malwilley malwilley deleted the malwilley/fix/recommended-event-event-id-tie branch November 21, 2023 17:18
Copy link

sentry-io bot commented Nov 21, 2023

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AssertionError: assert 'cccccccccccc...ccccccccccccc' == 'eeeeeeeeeeee...eeeeeeeeeeeee' pytest.runtest.protocol tests/sentry/api/endpoi... View Issue

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants