Fix Elastic::MigrationWorker current_migration (2nd attempt)
What does this MR do and why?
Related to #340478 (closed)
This is a second attempt to fix a bug with Advanced Search migrations, it was originally introduced in: !69958 (merged) and reverted in: !70405 (merged). The revert occurred because new migrations were not being picked up. The original fix changed from looking at completed migrations to not completed migrations. New migrations do not have a record in the migrations index so they would never be found.
This MR fixes the bug in Elastic::MigrationWorker#current_migration
by
- Removing the rescue from StandardError from
Elastic::MigrationRecord.load_versions
- Move
Elastic::MigrationWorker#current_migration
back to using completed migrations - Rescue from StandardError in
Elastic::MigrationWorker#current_migration
and return nil if an error occurs (note: this will catch Elasticsearch communication errors as well as errors where the migrations index is not found) - Move the migration index exists check in the
Elastic::MigrationWorker.perform
method to the top, this is to avoid an infinite loop if the index doesn't exist sincecurrent_migration
now returnsnil
in that scenario - update specs for new behavior
Screenshots or screen recordings
N/A
How to set up and validate locally
You must have your gdk setup to use Elasticsearch
- in rails console, run
Elastic::MigrationRecord.load_versions(completed: true)
- verify all of the migrations come back in an array
- run
gdk stop elasticsearch
- in rails console, run
Elastic::MigrationRecord.load_versions(completed: true)
- verify an exception is raised
It's hard to replicate a communication error while using the Elastic::MigrationWorker. However, you can add a breakpoint to simulate Elasticsearch being down before the current_migration call is run by taking down Elasticsearch at the breakpoint.
- run
gdk start elasticsearch
- add a breakpoint in the
Elastic::MigrationWorker.perform
method before themigration = current_migration
line
- in rails console, run
Elastic::MigrationWorker.new.perform
- when the breakpoint is hit, run
gdk stop elasticsearch
- verify that
current_migration
returnsnil
and doesn't raise an exception
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.