Skip to content

Remove Backup::Files subclasses

James Fargher requested to merge backup_task_cleanup into master

What does this MR do and why?

Since !79809 (merged) the extracted tasks have some remaining problems that prevent easy refactoring and changing of the implementation for backing up files in gitlab.

  • Removes all sub-classes of Backup::Files and uses the parent class directly. None of the Backup::Files sub-classes actually implement any special behaviour and when they do have tests, they only test that a configuration gets passed through or they re-test the implementation of Backup::Files which is already tested. I find these subclasses actually obscure what the backups are doing, adding an unused layer of abstraction. Removing this makes these tasks more declarative.
  • Hoists enabled and human_name out of the individual tasks into the task definitions. The implementation of these never depended on the task itself, they are both concerns of Backup::Manager.

How to set up and validate locally

No feature to test here. Just regression testing.

  1. Create a backup.

    $ bundle exec rake gitlab:backup:create
    ...
    2022-04-06 14:46:39 +1200 -- Backup 1649213182_2022_04_06_14.10.0-pre is done.
  2. Restore from the backup.

    $ bundle exec rake gitlab:backup:restore BACKUP=1649213182_2022_04_06_14.10.0-pre
    ...

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mayra Cabrera

Merge request reports

Loading