Not all hooks support all variables for scripts #420

Closed
opened 1 year ago by lasimik · 4 comments
lasimik commented 1 year ago

What I'm trying to do and why

I want to use borgmatic variables in hook scripts.

Steps to reproduce (if a bug)

Run a script (e.g. /etc/borgmatic/testscript.sh "{configuration_filename}" "{repository}" "{error}" "{output}") that echoes $1 thru 4$.

Do it from both the hooks after_backup and on_error.

EDIT: it may be relevant to run borgmatic by starting its borgmatic.service.

Actual behavior (if a bug)

The after_backup script does not resolve the variables $2 thru $4, producing "{repository}", "{error}" and "{output}" instead.

The same script run from on_error gives the desired output.

I have not tried other hooks.

Other notes / implementation ideas

All hooks should support all variables.

It would also be great if a successful full backup would supply a variable with the datetime of that backup ({last_successful_complete}?).

Environment

borgmatic version: 1.5.13

borgmatic installation method: pip3 install as per borgmatic page (probably, not fully sure)

Borg version: 1.1.16

Python version: 3.9.4

operating system and version: Manjaro Linux, Kernel 5.10 64Bit

#### What I'm trying to do and why I want to use borgmatic variables in hook scripts. #### Steps to reproduce (if a bug) Run a script (e.g. `/etc/borgmatic/testscript.sh "{configuration_filename}" "{repository}" "{error}" "{output}"`) that echoes `$1` thru `4$`. Do it from both the hooks `after_backup` and `on_error`. EDIT: it may be relevant to run borgmatic by starting its `borgmatic.service`. #### Actual behavior (if a bug) The `after_backup` script does not resolve the variables $2 thru $4, producing "{repository}", "{error}" and "{output}" instead. The same script run from `on_error` gives the desired output. I have not tried other hooks. #### Other notes / implementation ideas All hooks should support all variables. It would also be great if a successful full backup would supply a variable with the datetime of that backup (`{last_successful_complete}`?). #### Environment **borgmatic version:** 1.5.13 **borgmatic installation method:** pip3 install as per borgmatic [page](https://torsion.org/borgmatic/docs/how-to/set-up-backups/) (probably, not fully sure) **Borg version:** 1.1.16 **Python version:** 3.9.4 **operating system and version:** Manjaro Linux, Kernel 5.10 64Bit
Owner

Thank you for filing this. It seems reasonable for variable interpolation to apply to all of the command hooks. Variables like configuration_filename and repository would be generally available, but there may be a few cases where repository isn't available (like in before_everything/after_everything hooks). And error/output only make sense in on_error but not other hooks where an error isn't being processed.

So all hooks can't support all variables, but they can certainly get closer to it!

It would also be great if a successful full backup would supply a variable with the datetime of that backup ({last_successful_complete}?).

What's the use case here? In theory, if after_backup is triggered, a successful backup has just occurred "now".

Thank you for filing this. It seems reasonable for variable interpolation to apply to all of the command hooks. Variables like `configuration_filename` and `repository` would be generally available, but there may be a few cases where `repository` isn't available (like in `before_everything`/`after_everything` hooks). And `error`/`output` only make sense in `on_error` but not other hooks where an error isn't being processed. So all hooks can't support all variables, but they can certainly get closer to it! > It would also be great if a successful full backup would supply a variable with the datetime of that backup ({last_successful_complete}?). What's the use case here? In theory, if `after_backup` is triggered, a successful backup has just occurred "now".
Owner

Okay, I took a look at the code and it turns out that none of the command hooks are repository-specific. Meaning that they all run before (or after) individual repositories are looped over. So it doesn't look like any of the additional variables you're looking for would be available in the command hooks other than on_error.

So that leaves me with a couple more questions:

  1. Given that these variables aren't available, do you think some different behavior like replacing unavailable placeholders with empty string would make sense? E.g. replace {repository} with a blank string for hooks that don't support it?

  2. If you tell me more about what you're hoping to accomplish with these variables in your script, I might be able to come up with an alternate solution.

Thanks!

Okay, I took a look at the code and it turns out that *none* of the command hooks are repository-specific. Meaning that they all run before (or after) individual repositories are looped over. So it doesn't look like any of the additional variables you're looking for would be available in the command hooks other than `on_error`. So that leaves me with a couple more questions: 1. Given that these variables aren't available, do you think some different behavior like replacing unavailable placeholders with empty string would make sense? E.g. replace `{repository}` with a blank string for hooks that don't support it? 2. If you tell me more about what you're hoping to accomplish with these variables in your script, I might be able to come up with an alternate solution. Thanks!
Poster

Thanks for the replies. My thoughts re all variables in all hooks were that from the user's view there is less mental overhead (no individual checking for a variable - hook case), and from the programmer's view the same code block could be written once and for all.

An empty variable (eg "{error}" is "" everywhere except under on_error:) makes sense under these considerations. For extra user-friendliness you could supply in the documentation a small table (hooks * variables), where the cells give a symbol for "supported" or "not supported". A glance there is easier/faster than thinking through variables and runtime outcomes, as well as a heads-up for folks who did not think of the issue.

--
I have currently only experimented with a single repository, thus it's merely theory that running multiple configs with multiple repos could cause problems (e.g. crosstalk of variables between repos or configs).

That may become important in cases where different configs are called in the same borgmatic session, e.g. configs for different repos, or configs for a backup and for a thorough integrity check.

--
My use case for datetime is a file that stores the last successful backup datetime locally. I.e,

after_backup:
        - echo "$(borgmatic list --successful --last 1 --format {time} --no-color \
            | sed -n 2p) $(date +'%:z')" \
            > "/root/BackupUser@BackupServer/path/to/repository/last-successful-backup"

This is part of a setup I have described here, scroll down to "Example for Overdue Backups Alerts". (That was done on a test install that meanwhile has been deleted, the replacement has not yet its borgmatic, so further testing may not be possible for some time.)

Thanks for the replies. My thoughts re all variables in all hooks were that from the user's view there is less mental overhead (no individual checking for a variable - hook case), and from the programmer's view the same code block could be written once and for all. An empty variable (eg `"{error}"` is `""` everywhere except under `on_error:`) makes sense under these considerations. For extra user-friendliness you could supply in the documentation a small table (hooks * variables), where the cells give a symbol for "supported" or "not supported". A glance there is easier/faster than thinking through variables and runtime outcomes, as well as a heads-up for folks who did not think of the issue. -- I have currently only experimented with a single repository, thus it's merely theory that running multiple configs with multiple repos could cause problems (e.g. crosstalk of variables between repos or configs). That may become important in cases where different configs are called in the same borgmatic session, e.g. configs for different repos, or configs for a backup and for a thorough integrity check. -- My use case for `datetime` is a file that stores the last successful backup datetime locally. I.e, ``` after_backup: - echo "$(borgmatic list --successful --last 1 --format {time} --no-color \ | sed -n 2p) $(date +'%:z')" \ > "/root/BackupUser@BackupServer/path/to/repository/last-successful-backup" ``` This is part of a setup I have described [here](https://projects.torsion.org/lasimik/borgmatic_notifications/src/branch/master/Readme.md>), scroll down to "Example for Overdue Backups Alerts". (That was done on a test install that meanwhile has been deleted, the replacement has not yet its borgmatic, so further testing may not be possible for some time.)
Owner

I apologize for the lengthy delay here.

Okay, I took a look at the code and it turns out that none of the command hooks are repository-specific.

This is now no longer the case! As of borgmatic 1.6.0 (specificially #473), all before_ action and after_ action hooks are repository specific. Which means that repository as a variable is now available within those hooks. I also made borgmatic issue warnings in the console and log whenever an unknown/unsupported variable is present in a hook. (This change is in borgmatic 1.6.1.) I thought that was more idiomatic than stuffing a warning into the variable's value itself. Lastly, the documentation now lists which hooks support which variables—although not all in one place in a tabular format.

Given that you already have a work-around for timestamp of the last successful backup, I think I'll leave that part out for now.

Thanks!

I apologize for the lengthy delay here. > Okay, I took a look at the code and it turns out that none of the command hooks are repository-specific. This is now no longer the case! As of borgmatic 1.6.0 (specificially #473), all `before_` action and `after_` action hooks are repository specific. Which means that `repository` as a variable is now available within those hooks. I also made borgmatic issue warnings in the console and log whenever an unknown/unsupported variable is present in a hook. (This change is in borgmatic 1.6.1.) I thought that was more idiomatic than stuffing a warning into the variable's value itself. Lastly, the documentation now lists which hooks support which variables—although not all in one place in a tabular format. Given that you already have a work-around for timestamp of the last successful backup, I think I'll leave that part out for now. Thanks!
witten closed this issue 3 months ago
witten changed title from [Bug] Not all hooks support all variables for scripts to Not all hooks support all variables for scripts 3 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#420
Loading…
There is no content yet.