Skip to content

Conversation

@lobsterkatie
Copy link
Member

When seer doesn't find any near-enough neighbors, it sends back an empty list rather than None, and when it sends back results, none of them are ever None. We therefore don't need the None in the seer response typing.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 30, 2024
@codecov
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.83%. Comparing base (7263cac) to head (900106d).
Report is 14 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #69993    +/-   ##
========================================
  Coverage   79.83%   79.83%            
========================================
  Files        6495     6497     +2     
  Lines      288969   289126   +157     
  Branches    49765    49783    +18     
========================================
+ Hits       230686   230824   +138     
- Misses      57887    57906    +19     
  Partials      396      396            
Files Coverage Δ
...y/api/endpoints/group_similar_issues_embeddings.py 100.00% <100.00%> (ø)
src/sentry/seer/utils.py 100.00% <100.00%> (ø)

... and 22 files with indirect coverage changes

@lobsterkatie lobsterkatie marked this pull request as ready for review April 30, 2024 20:52
@lobsterkatie lobsterkatie requested a review from jangjodi April 30, 2024 20:53
@lobsterkatie lobsterkatie merged commit b599f3a into master Apr 30, 2024
@lobsterkatie lobsterkatie deleted the kmclb-remove-None-from-SimilarIssuesEmbeddingsResponse-type branch April 30, 2024 21:20
lobsterkatie added a commit that referenced this pull request May 1, 2024
This is a handful of small fixes pulled out of a few upcoming PRs, in order to make them easier to review.

Changes:

- Switch from using inheritance and `total=False` to using `NotRequired` for optional properties of `SimilarIssuesEmbeddingsRequest`, so everything can be in one class.
- Add exception types thrown by deserialization to the `try-except` block surrounding the parsing of seer response data, and add a log to track if/when things go wrong. Also simplify returning an empty result-set from the `except` block.
- Simplify a few places where we were using `update` to add a single key-value pair to a dictionary.
- Remove an unnecessary check for the existence of entries in the seer result-set. This should have been part of #69993, but it got missed.
- Rename `response`/`responses` in  `get_formatted_results` to `similar_issue_data`/`similar_issues_data` in preparation for changing some seer data datatypes.
- Add a TODO to track the fact that we need to at some point let seer know when it's recommending groups which no longer exist.
- Fix the data in a `get_similar_issues_embeddings` test to match the `SimilarIssuesEmbeddingsData` type. (It was given in terms of message and stacktrace similarity rather than distance.)
- Add types to similar issues endpoint helpers.
- Prepend `get_value_if_exists` with an underscore since it's an internal helper.
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2024
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.

3 participants