No way to conditionally list repos #307

Closed
opened 2020-05-02 02:21:57 +00:00 by rofer · 8 comments

What I'm trying to do and why

I have a repo setup for an offline backup to a drive that's only occasionally connected to my computer. By using a before_backup command like:

findmnt /run/media/rofer/OfflineBackup > /dev/null || exit 75

I'm able to use borgmatic with or without this device connected and most things work fine. However, whenever I run borgmatic list without this drive connected I get an error complaining it couldn't be found.

I understand why it wouldn't make sense for my before_backup command to run during borgmatic list, but it would be nice to have a similar way for a configured command to be able to check if the drive is connected. In the simplest case there could just be a before_list command.

Better still would be some kind of before_use command which runs before any command would look for the repo like check, backup, and list. That way I could put my findmnt command in one place and have all of these commands work as expected.

If something like my before_use idea would be acceptable I can probably find the time to implement it.

Environment

borgmatic version: 1.5.1

Use sudo borgmatic --version or sudo pip show borgmatic | grep ^Version

borgmatic installation method: Fedora 31 package

Borg version: borg 1.1.11

Use sudo borg --version

Python version: Python 3.7.7

Use python3 --version

operating system and version: Fedora 31 (amd64)

#### What I'm trying to do and why I have a repo setup for an offline backup to a drive that's only occasionally connected to my computer. By using a before_backup command like: ``` findmnt /run/media/rofer/OfflineBackup > /dev/null || exit 75 ``` I'm able to use borgmatic with or without this device connected and most things work fine. However, whenever I run `borgmatic list` without this drive connected I get an error complaining it couldn't be found. I understand why it wouldn't make sense for my before_backup command to run during `borgmatic list`, but it would be nice to have a similar way for a configured command to be able to check if the drive is connected. In the simplest case there could just be a before_list command. Better still would be some kind of before_use command which runs before any command would look for the repo like check, backup, and list. That way I could put my findmnt command in one place and have all of these commands work as expected. If something like my before_use idea would be acceptable I can probably find the time to implement it. #### Environment **borgmatic version:** 1.5.1 Use `sudo borgmatic --version` or `sudo pip show borgmatic | grep ^Version` **borgmatic installation method:** Fedora 31 package **Borg version:** borg 1.1.11 Use `sudo borg --version` **Python version:** Python 3.7.7 Use `python3 --version` **operating system and version:** Fedora 31 (amd64)
Owner

Interesting idea! There's already a before_everything hook, but it currently has some limitations:

  1. It runs before all configuration files are interpreted.
  2. It doesn't yet support the soft failure feature you're looking to use.
  3. It only runs when there's a create action present.

So if that last restriction was lifted, and before_everything was changed to run for all actions, and it gained soft failure support, do you think that would work for your needs?

Interesting idea! There's already a [`before_everything` hook](https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/#preparation-and-cleanup-hooks), but it currently has some [limitations](https://torsion.org/borgmatic/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server/#caveats-and-details): 1. It runs before all configuration files are interpreted. 2. It doesn't yet support the soft failure feature you're looking to use. 3. It only runs when there's a `create` action present. So if that last restriction was lifted, and `before_everything` was changed to run for all actions, and it gained soft failure support, do you think that would work for your needs?
Author

I think that would work as long as it was also changed to run per-configuration rather than aggregated together (so list would still list all the repos I expect to be available). However, I can imagine all those changes together might break existing workflows so it might be easier to add a new hook instead.

Maybe before_repo? It could even be a hook that exists specifically for this case of checking that the location is ready to be used as I imagine that's somewhat common. Maybe something like repo_condition.

I think that would work as long as it was also changed to run per-configuration rather than aggregated together (so list would still list all the repos I expect to be available). However, I can imagine all those changes together might break existing workflows so it might be easier to add a new hook instead. Maybe before_repo? It could even be a hook that exists specifically for this case of checking that the location is ready to be used as I imagine that's somewhat common. Maybe something like repo_condition.
Owner

Gotcha. In that case, what do you think of before_actions (and after_actions), as that would be in keeping with the terminology used elsewhere? It could be used for repo condition checks, or for any other commands the user wants run before all actions (for a particular configuration file).

Gotcha. In that case, what do you think of `before_actions` (and `after_actions`), as that would be in keeping with the terminology used elsewhere? It could be used for repo condition checks, or for any other commands the user wants run before all actions (for a particular configuration file).
Author

I trust you know what fits much better than I do. before_actions and after_actions sound good to me. I'll start working on the patch.

I trust you know what fits much better than I do. `before_actions` and `after_actions` sound good to me. I'll start working on the patch.
Owner

Awesome!

Awesome!
witten added the
waiting for response
label 2020-05-26 16:47:01 +00:00
Author

Ok, it took me way longer than I intended to get to this, but I think I have something that should work.

What I'm wondering now is if there are any relevant tests I can update to include the new hooks. I couldn't find any, but I could try my hand at writing some if you can point me in the right direciton.

Ok, it took me way longer than I intended to get to this, but I think I have something that should work. What I'm wondering now is if there are any relevant tests I can update to include the new hooks. I couldn't find any, but I could try my hand at writing some if you can point me in the right direciton.
Owner

Cool! I'm glad to hear you've made progress.

If you've added your calls to the new hooks within borgmatic/commands/borgmatic.py in the run_configuration() function, then the relevant tests would be the test_run_configuration_*() tests in tests/unit/commands/test_borgmatic.py.

The tests there are kind of obnoxious (lots of mocking), but they do exercise several of the existing hooks.. Look to the names of the test functions. If you run tox, you'll get an idea of whether your changes have broken any of the existing tests.

Let me know if you need any more help with testing. There's also more here: https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/

Cool! I'm glad to hear you've made progress. If you've added your calls to the new hooks within `borgmatic/commands/borgmatic.py` in the `run_configuration()` function, then the relevant tests would be the `test_run_configuration_*()` tests in `tests/unit/commands/test_borgmatic.py`. The tests there are kind of obnoxious (lots of mocking), but they do exercise several of the existing hooks.. Look to the names of the test functions. If you run `tox`, you'll get an idea of whether your changes have broken any of the existing tests. Let me know if you need any more help with testing. There's also more here: https://torsion.org/borgmatic/docs/how-to/develop-on-borgmatic/
Owner

Closing due to inactivity. Please feel free to reopen if you get back to the pull request. I'm happy to help out with tests!

Closing due to inactivity. Please feel free to reopen if you get back to the pull request. I'm happy to help out with tests!
witten removed the
waiting for response
label 2021-08-31 23:40:37 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#307
No description provided.