chore(similarity): Add try catch around insert grouping record #1184
Conversation
src/seer/grouping/grouping.py
Outdated
| @@ -498,15 +504,26 @@ def insert_new_grouping_record( | |||
| error_type=issue.exception_type, | |||
| ).to_db_model() | |||
| session.add(new_record) | |||
|
|
|||
| try: | |||
| session.flush() | |||
There was a problem hiding this comment.
I initially thought we didn't want to rollback the entire session, but that was incorrect. I created a new session to rollback, but would it also work if I kept the session parameter and did rollback since there were no other adds?
src/seer/grouping/grouping.py
Outdated
| "input_hash": issue.hash, | ||
| } | ||
|
|
||
| if existing_record is None: |
There was a problem hiding this comment.
can we modify this to return early if existing record is not none so we don't get so much arrowing? https://blog.codinghorror.com/flattening-arrow-code/
src/seer/grouping/grouping.py
Outdated
| try: | ||
| session.commit() | ||
| except IntegrityError: | ||
| session.rollback() |
There was a problem hiding this comment.
what does rollback do? do we need it here?
src/seer/grouping/grouping.py
Outdated
| existing_record = ( | ||
| session.query(DbGroupingRecord) | ||
| .filter_by(hash=issue.hash, project_id=issue.project_id) | ||
| .first() | ||
| ) | ||
| extra["existing_hash"] = existing_record.hash | ||
| logger.info( | ||
| "group_already_exists_in_seer_db", | ||
| extra=extra, | ||
| ) |
There was a problem hiding this comment.
i'm wondering if this is necessary or if we can skip not having this log -- in theory we should be able to see the group hashes linked on the sentry side, right?
|
@JoshFerge I updated this PR to use on_conflict_do_nothing, instead of logging and doing the try catch. Let me know if that makes sense! |
Add
on_conflict_do_nothingfor inserting group recordsfixes SEER-9Y