List merging #531

Closed
opened 3 months ago by ShadowRylander Β· 15 comments

What I'm trying to do and why

Hello!

Would it be possible to implement list merging? If possible, merge the lists from the includes, then remove duplicates; or if not making that the default to avoid breaking current configs, have an option in the config with the include directive that enables it, like borgmatic_settings?

Thank you kindly for your consideration on the matter!

#### What I'm trying to do and why Hello! Would it be possible to implement list merging? If possible, merge the lists from the includes, then remove duplicates; or if not making that the default to avoid breaking current configs, have an option in the config with the include directive that enables it, like `borgmatic_settings`? Thank you kindly for your consideration on the matter!
Owner

Thanks for taking the time to file this! Could you talk a little more about your use case? E.g., what you're trying to accomplish by merging lists? I think elsewhere you mentioned wanting to merge before_backup hooks? What's the rationale there? And what's the goal with removing duplicates?

Also, given that order often matters with lists like command hooks (since they are running arbitrary commands), do you have thoughts about how to do ordering of the merged lists? Perhaps simple appending with included values tacked onto the end of the list?

I'm not super concerned about breaking current configs in this particular case, since 1.6.0 already was a "breaking change" (and documented as such) in that it introduced include merging.

Thanks for taking the time to file this! Could you talk a little more about your use case? E.g., what you're trying to accomplish by merging lists? I think elsewhere you mentioned wanting to merge `before_backup` hooks? What's the rationale there? And what's the goal with removing duplicates? Also, given that order often matters with lists like command hooks (since they are running arbitrary commands), do you have thoughts about how to do ordering of the merged lists? Perhaps simple appending with included values tacked onto the end of the list? I'm not *super* concerned about breaking current configs in this particular case, since 1.6.0 already was a "breaking change" (and documented as such) in that it introduced include merging.

Alright, so these are parts of my configs:


<<: !include /etc/borgmatic/shared.yaml

location:
    source_directories:
        - /chimchar/oreo
    repositories:
        - /oreo
    hooks:
        before_backup:
            - findmnt /chimchar/oreo > /dev/null || exit 75
            - findmnt /oreo > /dev/null || exit 75

<<: !include /etc/borgmatic/shared.yaml

location:
    source_directories:
        - /chimchar/oreo
    repositories:
        - 9237@usw-s009.rsync.net/./oreo-rsync
    hooks:
        before_backup:
            - findmnt /chimchar/oreo > /dev/null || exit 75
            - ping -qc 1 usw-s009.rsync.net || exit 75
    remote_path: borg1

The reason I want to merge lists is so that I don't have to repeat, for example, findmnt /chimchar/oreo > /dev/null || exit 75 and /chimchar/oreo. Yes, I know it's just one line that's unlikely to change, but I'm a very forgetful person! πŸ˜…

Alright, so these are parts of my configs: ```yaml <<: !include /etc/borgmatic/shared.yaml location: source_directories: - /chimchar/oreo repositories: - /oreo hooks: before_backup: - findmnt /chimchar/oreo > /dev/null || exit 75 - findmnt /oreo > /dev/null || exit 75 ``` ```yaml <<: !include /etc/borgmatic/shared.yaml location: source_directories: - /chimchar/oreo repositories: - 9237@usw-s009.rsync.net/./oreo-rsync hooks: before_backup: - findmnt /chimchar/oreo > /dev/null || exit 75 - ping -qc 1 usw-s009.rsync.net || exit 75 remote_path: borg1 ``` The reason I want to merge lists is so that I don't have to repeat, for example, `findmnt /chimchar/oreo > /dev/null || exit 75` and `/chimchar/oreo`. Yes, I know it's just one line that's unlikely to change, but I'm a very forgetful person! πŸ˜…
ShadowRylander closed this issue 3 months ago

Oops...

Oops...
ShadowRylander reopened this issue 3 months ago
Owner

Got it, that makes sense. Any thoughts on the ordering question or the need for deduplication? Would it be a problem if one of the findmnt commands ran twice?

Got it, that makes sense. Any thoughts on the ordering question or the need for deduplication? Would it be a problem if one of the `findmnt` commands ran twice?

Oh, right; forgot to answer the rest...

If there were settings for merging lists, for example, like borgmatic_settings.merge_lists.before, then list items could be prepended depending on what oreder the include directives are in.

Regarding removing duplicates, running hooks twice, for example, may not be the best idea.

Oh, right; forgot to answer the rest... If there were settings for merging lists, for example, like `borgmatic_settings.merge_lists.before`, then list items could be prepended depending on what oreder the include directives are in. Regarding removing duplicates, running hooks twice, for example, may not be the best idea.

findmnt would be fine, but I also run git pull --all; yes, it is also a relatively safe command, but if I were to run something like git submodule update --init --recursive --remote --force, to update and initialize new submodules, for example, then that might be a problem.

`findmnt` would be fine, but I also run `git pull --all`; yes, it is also a relatively safe command, but if I were to run something like `git submodule update --init --recursive --remote --force`, to update and initialize new submodules, for example, then that might be a problem.

Oh, and regarding a possible borgmatic_settings option, these probably shouldn't be merged. Although that might be complicated...

Oh, and regarding a possible `borgmatic_settings` option, these probably shouldn't be merged. Although that might be complicated...
Owner

If there were settings for merging lists, for example, like borgmatic_settings.merge_lists.before, then list items could be prepended depending on what order the include directives are in.

Gotcha!

And that's a fair point in regards to deduping, although I can also conceive of potential issues with deciding to deduplicate away particular commands where that's not expected. For instance, imagine merging this very contrived example:

before_backup:
    - lock all databases
    - dump database 1
    - unlock all databases

... with this:

before_backup:
    - lock all databases
    - dump database 2
    - unlock all databases

If you deduplicate that, you'll get rid of database locking/unlocking for one of the dumps. It seems safer, to me, to rely on the user to figure out what needs deduplication "manually" by virtue of what they chose to merge.

> If there were settings for merging lists, for example, like borgmatic_settings.merge_lists.before, then list items could be prepended depending on what order the include directives are in. Gotcha! And that's a fair point in regards to deduping, although I can also conceive of potential issues with deciding to deduplicate away particular commands where that's not expected. For instance, imagine merging this very contrived example: ``` before_backup: - lock all databases - dump database 1 - unlock all databases ``` ... with this: ``` before_backup: - lock all databases - dump database 2 - unlock all databases ``` If you deduplicate that, you'll get rid of database locking/unlocking for one of the dumps. It seems safer, to me, to rely on the user to figure out what needs deduplication "manually" by virtue of what they chose to merge.

What if you get rid on the same items one after the other? So this would be fine:

before_backup:
    - lock all databases
    - dump database 1
    - unlock all databases
    - lock all databases
    - dump database 2
    - unlock all databases

But the combined result of this would be deduplicated, if prepended (can't quite think of a proper example right now):

before_backup:
    - lock all databases
before_backup:
    - lock all databases
    - dump database 2
    - unlock all databases
What if you get rid on the same items one after the other? So this would be fine: ```yaml before_backup: - lock all databases - dump database 1 - unlock all databases - lock all databases - dump database 2 - unlock all databases ``` But the combined result of this would be deduplicated, if prepended (can't quite think of a proper example right now): ```yaml before_backup: - lock all databases ``` ```yaml before_backup: - lock all databases - dump database 2 - unlock all databases ```
Owner

Interesting! That might work in that scenario. I'll have to think about whether there are other scenarios where it could cause issues.

Interesting! That might work in that scenario. I'll have to think about whether there are other scenarios where it could cause issues.

Most likely; my ideas here aren't particularly thorough.

Most likely; my ideas here aren't particularly thorough.
Owner

Neither are mine. πŸ˜„

Neither are mine. πŸ˜„
Owner

This is done in master, and will be part of the next release! (It's enabled unconditionally with no way to disable.) I didn't tackle anything with deduplication, as that seemed like a can of worms. But for now it's just doing simple appending: For any colliding list, the included values come first. And then those are appended with values from the config file doing the including.

Thanks again for suggesting the feature!

This is done in master, and will be part of the next release! (It's enabled unconditionally with no way to disable.) I didn't tackle anything with deduplication, as that seemed like a can of worms. But for now it's just doing simple appending: For any colliding list, the included values come first. And then those are appended with values from the config file doing the including. Thanks again for suggesting the feature!
witten closed this issue 3 months ago

Got it; thanks for all the help! Really appreciate it! 😸

Got it; thanks for all the help! Really appreciate it! 😸
Owner

Released in borgmatic 1.6.1!

Released in borgmatic 1.6.1!
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#531
Loading…
There is no content yet.