[keyserver] only wrap migration in transaction if specified in migrationType
ClosedPublic
Actions

Authored by will on Fri, Aug 30, 8:29 AM.

Details

Reviewers

varun
ashoat

Commits

rCOMM13bc5582d73e: [keyserver] only wrap migration in transaction if specified in migrationType

Summary

If the migration type is wrap_in_transaction_and_block_requests, we wrap the migration in a transaction. Only if it was wrapped in transaction do we run rollback if the query failed.

Depends on D13209

Test Plan

I console logged and made sure that transaction and rollback queries were only being run if the migration in migrations specified wrapping in a transaction.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

will created this revision.Fri, Aug 30, 8:29 AM

Herald added a subscriber: tomek. · View Herald TranscriptFri, Aug 30, 8:29 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Fri, Aug 30, 8:40 AM

Harbormaster failed remote builds in B31372: Diff 43816!

Harbormaster completed remote builds in B31372: Diff 43816.Fri, Aug 30, 9:00 AM

will requested review of this revision.Fri, Aug 30, 9:00 AM

will added 1 blocking reviewer(s): ashoat.Tue, Sep 3, 8:01 AM

ashoat accepted this revision.Wed, Sep 4, 11:20 PM

ashoat added inline comments.

keyserver/src/database/migrations.js
51–52 ↗	(On Diff #43816)	Let's avoid copy-pasting these two lines. Can't we just individually wrap `startTransaction` and `commitTransaction` in the conditional?

This revision is now accepted and ready to land.Wed, Sep 4, 11:20 PM

will added inline comments.Thu, Sep 5, 6:51 AM

keyserver/src/database/migrations.js
51–52 ↗	(On Diff #43816)	That makes sense! I originally had them both individually wrapped but thought this looked cleaner. I'll go by the maxim of reducing copy-paste of non-conditionals from now on