borgmatic-collective/borgmatic
borgmatic-collective
/
borgmatic
15
57
Fork 40
Code Issues 54 Pull Requests 1 Actions Packages Releases 121 Activity

Remove configuration sections entirely (except for deprecated support) #721

New Issue
Closed
opened 2023-06-29 20:33:07 +00:00 by witten · 11 comments
witten commented 2023-06-29 20:33:07 +00:00
Owner
Copy Link

What I'm trying to do and why

Here's a common UX annoyance with borgmatic: You want to add a new configuration option to your borgmatic config file because your backup requirements have changed. So you open your config file and uncomment the new option or type it in. Then you run borgmatic, only to get an obnoxious error about unexpected this or that—because you forgot to scroll way up and also uncomment the corresponding section header (location:, storage:, retention:, consistency:, hooks:, etc.). You swear under your breath, uncomment the header, and move on with your task. Or worse, if you're a new user and don't know what the error means, you file a ticket or just stop using borgmatic entirely.

Similarly: Let's say your backup needs change and that option you used to need is no longer necessary. So you open your borgmatic config file, comment out the option, and run borgmatic, only to get an annoying error message about hooks: or whatever section having no values—because you just commented out its last remaining option. You swear, this time not under your breath, comment out hooks:, and move on with your day.

But what if borgmatic could do away with these recurring annoyances and eliminate section headers entirely? Just for context: They were originally added for organization purposes back when borgmatic had way fewer options. But one might argue that any organization they provide now is pretty minimal.

Pop quiz: What section is the lock_wait option in? Do you know? No? Well I don't either, and that's kind of my point. If the section headers aren't really providing much value in the way of organization, maybe they need to go.

I don't propose eliminating other hierarchy besides the section headers in the configuration file. Usually other hierarchy (e.g. postgresql_databases:) is much more localized, and so you're less likely to forget to comment/uncomment it when changing a sub-option.

What this might look like

If this is your current config file ...

location:
    source_directories:
        - /home
        - /etc

    repositories:
        - path: ssh://k8pDxu32@k8pDxu32.repo.borgbase.com/./repo
          label: borgbase
        - path: /var/lib/backups/local.borg
          label: local

retention:
    keep_daily: 7
    keep_weekly: 4
    keep_monthly: 6

consistency:
    checks:
        - name: repository
        - name: archives
          frequency: 2 weeks

hooks:
    before_backup:
        - prepare-for-backup.sh

    postgresql_databases:
        - name: users

    healthchecks: https://hc-ping.com/be067061-cf96-4412-8eae-62b0c50d6a8c

... this might be the same configuration after the proposed changes are implemented:

source_directories:
    - /home
    - /etc

repositories:
    - path: ssh://k8pDxu32@k8pDxu32.repo.borgbase.com/./repo
      label: borgbase
    - path: /var/lib/backups/local.borg
      label: local

keep_daily: 7
keep_weekly: 4
keep_monthly: 6

checks:
    - name: repository
    - name: archives
      frequency: 2 weeks

before_backup:
    - prepare-for-backup.sh

postgresql_databases:
    - name: users

healthchecks: https://hc-ping.com/be067061-cf96-4412-8eae-62b0c50d6a8c

Other notes / implementation ideas

In terms of implementation, I propose deprecating but retaining support for section headers (so, not making a breaking change now) and simply normalizing any old-style configs as they're loaded. Under the hood, normalize() could slurp in any section config and move it to the top-level, effectively merging all the sections together.

There is at least one case where there'd be a collision: retention prefix and consistency prefix. My suggestion there is to silently merge them unless both are set and the values differ, in which case borgmatic could error. (This would technically be a minor breaking change. Oopsie daisy.)

There might also be an option here or there that could do with renaming once it's pulled into global scope. (output color comes to mind.)

Much of the borgmatic code would also need refactoring not to expect to pull options out of sections. This would be a very large and wide-ranging change, but hopefully a mostly mechanical one.

Given that 1.8.0 is the next release, this seems like a natural time to pull off this particular band-aid.

#### What I'm trying to do and why Here's a common UX annoyance with borgmatic: You want to add a new configuration option to your borgmatic config file because your backup requirements have changed. So you open your config file and uncomment the new option or type it in. Then you run borgmatic, only to get an obnoxious error about unexpected this or that—because you forgot to scroll way up and also uncomment the corresponding section header (`location:`, `storage:`, `retention:`, `consistency:`, `hooks:`, etc.). You swear under your breath, uncomment the header, and move on with your task. Or worse, if you're a new user and don't know what the error means, you file a ticket or just stop using borgmatic entirely. Similarly: Let's say your backup needs change and that option you *used* to need is no longer necessary. So you open your borgmatic config file, comment out the option, and run borgmatic, only to get an annoying error message about `hooks:` or whatever section having no values—because you just commented out its last remaining option. You swear, this time not under your breath, comment out `hooks:`, and move on with your day. But what if borgmatic could do away with these recurring annoyances and eliminate section headers entirely? Just for context: They were originally added for organization purposes back when borgmatic had way fewer options. But one might argue that any organization they provide now is pretty minimal. Pop quiz: What section is the `lock_wait` option in? Do you know? *No?* Well I don't either, and that's kind of my point. If the section headers aren't really providing much value in the way of organization, maybe they need to go. I *don't* propose eliminating other hierarchy besides the section headers in the configuration file. Usually other hierarchy (e.g. `postgresql_databases:`) is much more localized, and so you're less likely to forget to comment/uncomment it when changing a sub-option. #### What this might look like If this is your current config file ... ```yaml location: source_directories: - /home - /etc repositories: - path: ssh://k8pDxu32@k8pDxu32.repo.borgbase.com/./repo label: borgbase - path: /var/lib/backups/local.borg label: local retention: keep_daily: 7 keep_weekly: 4 keep_monthly: 6 consistency: checks: - name: repository - name: archives frequency: 2 weeks hooks: before_backup: - prepare-for-backup.sh postgresql_databases: - name: users healthchecks: https://hc-ping.com/be067061-cf96-4412-8eae-62b0c50d6a8c ``` ... this might be the same configuration after the proposed changes are implemented: ```yaml source_directories: - /home - /etc repositories: - path: ssh://k8pDxu32@k8pDxu32.repo.borgbase.com/./repo label: borgbase - path: /var/lib/backups/local.borg label: local keep_daily: 7 keep_weekly: 4 keep_monthly: 6 checks: - name: repository - name: archives frequency: 2 weeks before_backup: - prepare-for-backup.sh postgresql_databases: - name: users healthchecks: https://hc-ping.com/be067061-cf96-4412-8eae-62b0c50d6a8c ``` #### Other notes / implementation ideas In terms of implementation, I propose deprecating but retaining support for section headers (so, *not* making a breaking change now) and simply normalizing any old-style configs as they're loaded. Under the hood, `normalize()` could slurp in any section config and move it to the top-level, effectively merging all the sections together. There is at least one case where there'd be a collision: `retention` `prefix` and `consistency` `prefix`. My suggestion there is to silently merge them unless both are set and the values differ, in which case borgmatic could error. (This would technically be a minor breaking change. Oopsie daisy.) There might also be an option here or there that could do with renaming once it's pulled into global scope. (`output` `color` comes to mind.) Much of the borgmatic code would also need refactoring not to expect to pull options out of sections. This would be a very large and wide-ranging change, but hopefully a mostly mechanical one. Given that 1.8.0 is the next release, this seems like a natural time to pull off this particular band-aid.
🚀 1
witten changed title from Remove configuration sections entirely to Remove configuration sections entirely (except for deprecated support) 2023-06-29 20:43:10 +00:00
witten referenced this issue from a commit 2023-07-09 06:14:57 +00:00
Remove sections (#721).
witten commented 2023-07-14 03:11:36 +00:00
Author
Owner
Copy Link

Merged into main and will be part of the next release.

Merged into main and will be part of the next release.
witten commented 2023-07-19 06:01:53 +00:00
Author
Owner
Copy Link

Released in borgmatic 1.8.0!

Released in borgmatic 1.8.0!
rodrigodev commented 2023-07-23 07:08:11 +00:00
Copy Link

Hi, I think this is a troublesome change for people like me that rely on those sections to be able to use multiple << include merges in a single configuration file (e.g. to have separate different combinations of retention options and hooks).

Is there any possibility to make them permanently optional instead of deprecating them? Just wondering.

Hi, I think this is a troublesome change for people like me that rely on those sections to be able to use multiple << include merges in a single configuration file (e.g. to have separate different combinations of retention options and hooks). Is there any possibility to make them permanently optional instead of deprecating them? Just wondering.
👍 1
witten commented 2023-07-23 16:09:44 +00:00
Author
Owner
Copy Link

Thanks for chiming in with your experience here! Honestly I was a little worried about this use case with the changes in this ticket, so it's good to hear from someone who actually has this need. As an alternative to the sections for multiple << includes, what do you think of borgmatic actually supporting an include that pulls in multiple files? Something like this (although I haven't vetted the feasibility):

<<: !include common1.yaml common2.yaml common3.yaml

Thoughts? Other ideas?

Thanks for chiming in with your experience here! Honestly I was a little worried about this use case with the changes in this ticket, so it's good to hear from someone who actually has this need. As an alternative to the sections for multiple << includes, what do you think of borgmatic actually supporting an include that pulls in multiple files? Something like this (although I haven't vetted the feasibility): ```python <<: !include common1.yaml common2.yaml common3.yaml ``` Thoughts? Other ideas?
👍 2
rodrigodev commented 2023-07-23 23:40:23 +00:00
Copy Link

Oh wow, that idea actually sounds great to me, I was thinking of working around the issue by using yq in a similar fashion (although in a way that would require me to do some preprocessing to get the actual final configuration files, not that great).

If I were able to do what you just described (and in a way where changes are applied in listed order like I assume is the case), I think many of my configuration files could simply contain a single include line and that would be amazing.

Oh wow, that idea actually sounds great to me, I was thinking of working around the issue by using [yq](https://github.com/mikefarah/yq) in a similar fashion (although in a way that would require me to do some preprocessing to get the actual final configuration files, not that great). If I were able to do what you just described (and in a way where changes are applied in listed order like I assume is the case), I think many of my configuration files could simply contain a single include line and that would be amazing.
witten commented 2023-07-24 04:07:02 +00:00
Author
Owner
Copy Link

Great! I broke off that feature into #732, which you are welcome to follow! (Also.. TIL I learned about yq! 😄)

Great! I broke off that feature into #732, which you are welcome to follow! (Also.. TIL I learned about yq! 😄)
rodrigodev commented 2023-07-24 06:56:41 +00:00
Copy Link

Thank you so much, definitely looking forward to it! 🙂

Thank you so much, definitely looking forward to it! 🙂
👍 1
witten commented 2023-08-02 04:51:01 +00:00
Author
Owner
Copy Link

That multiple include feature in #732 is now implemented in main and will be part of the next release! Documentation here: https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#multiple-merge-includes

That multiple include feature in #732 is now implemented in main and will be part of the next release! Documentation here: https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#multiple-merge-includes
🎉 1
witten commented 2023-08-04 04:59:20 +00:00
Author
Owner
Copy Link

And #732 (multiple includes) has just been released in borgmatic 1.8.1! Let me know how it works out for you.

And #732 (multiple includes) has just been released in borgmatic 1.8.1! Let me know how it works out for you.
rodrigodev commented 2023-10-12 06:53:50 +00:00
Copy Link

Sorry for the very late reply, but I was able to update all of my configuration just recently, making use of the new multi-include feature, and it indeed works pretty well. Thanks a lot!

Sorry for the very late reply, but I was able to update all of my configuration just recently, making use of the new multi-include feature, and it indeed works pretty well. Thanks a lot!
witten commented 2023-10-12 16:46:17 +00:00
Author
Owner
Copy Link

Awesome, glad to hear it!

Awesome, glad to hear it!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
main
spot-check
1.8.10
1.8.9
1.8.8
1.8.7
1.8.6
1.8.5
1.8.4
1.8.3
1.8.2
1.8.1
1.8.0
1.7.15
1.7.14
1.7.13
1.7.12
1.7.11
1.7.10
1.7.9
1.7.8
1.7.7
1.7.6
1.7.5
1.7.4
1.7.3
1.7.2
1.7.1
1.7.0
1.6.6
1.6.5
1.6.4
1.6.3
1.6.2
1.6.1
1.6.0
1.5.24
1.5.23
1.5.22
1.5.21
1.5.20
1.5.19
1.5.18
1.5.17
1.5.16
1.5.15
1.5.14
1.5.13
1.5.12
1.5.11
1.5.10
1.5.9
1.5.8
1.5.7
1.5.7.dev0
1.5.6
1.5.5
1.5.4
1.5.3
1.5.2
1.5.1
1.5.0
1.4.22
1.4.21
1.4.20
1.4.19
1.4.18
1.4.17
1.4.16
1.4.15
1.4.14
1.4.13
1.4.12
1.4.11
1.4.10
1.4.9
1.4.8
1.4.7
1.4.6
1.4.5
1.4.4
1.4.3
1.4.2
1.4.1
1.4.0
1.3.26
1.3.25
1.3.24
1.3.23
1.3.22
1.3.21
1.3.20
1.3.19
1.3.18
1.3.17
1.3.16
1.3.15
1.3.14
1.3.13
1.3.12
1.3.11
1.3.10
1.3.9
1.3.8
1.3.7
1.3.6
1.3.5
1.3.4
1.3.3
1.3.2
1.3.1
1.3.0
1.2.18
1.2.17
1.2.16
1.2.15
1.2.14
1.2.13
1.2.12
1.2.11
1.2.10
1.2.9
1.2.8
1.2.7
1.2.6
1.2.5
1.2.4
1.2.3
1.2.2
1.2.1
1.2.0
1.1.15
1.1.14
1.1.13
1.1.12
1.1.11
1.1.10
1.1.9
1.1.7
1.1.6
1.1.5
1.1.4
1.1.3
1.1.2
1.1.1
1.1.0
1.0.3
1.0.2
1.0.1
1.0.0
0.1.8
0.1.7
0.1.6
0.1.5
0.1.4
0.1.3
0.1.2
0.1.1
0.1.0
0.0.7
0.0.6
0.0.5
0.0.4
0.0.3
0.0.2
Labels
Clear labels   
bug

   
data loss

   
design finalized

   
good first issue

   
new feature area

   
question / support

   
security

   
waiting for response
No Label
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
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#721
No description provided.
Delete Branch "%!s(<nil>)"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?