refactor(robot-server): consolidate DB transactions, fix a max analyses length bug #14904
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sanni-t
requested review from
a team,
jbleon95,
Elyorcv and
SyntaxColoring
and removed request for
a team and
Elyorcv
sanni-t
deleted the
AUTH-347-refactor-consolidate-db-transactions-when-adding-deleting-analyses
branch
Carlos-fernandez
pushed a commit
that referenced
this pull request
May 20, 2024
…es length bug (#14904) Closes AUTH-347 # Overview #14885 added the feature to limit number of analyses we store in DB. In [this](#14885 (comment)) comment, @SyntaxColoring pointed out that we should consolidate the DB transactions for better performance, so that's what this PR does. Also fixes a bug where if the existing number of analyses in the DB was 3 and we were to add another analysis, then the formula for getting the analysis IDs to delete would result in `analysis_ids[:-1]` and it would delete all analyses except last one. # Test Plan - Tested the cases mentioned in #14885 - Tested the bug case # Risk assessment Low. Refactor + small bug fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes AUTH-347
Overview
#14885 added the feature to limit number of analyses we store in DB. In this comment, @SyntaxColoring pointed out that we should consolidate the DB transactions for better performance, so that's what this PR does.
Also fixes a bug where if the existing number of analyses in the DB was 3 and we were to add another analysis, then the formula for getting the analysis IDs to delete would result in
analysis_ids[:-1]and it would delete all analyses except last one.Test Plan
Risk assessment
Low. Refactor + small bug fix