List merging #531

New Issue

Closed

opened 2022-05-10 01:26:06 +00:00 by ShadowRylander · 15 comments

ShadowRylander commented 2022-05-10 01:26:06 +00:00

Copy Link

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!

witten commented 2022-05-10 02:38:40 +00:00

Owner

Copy Link

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.

ShadowRylander commented 2022-05-10 03:18:17 +00:00

Author

Copy Link

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 2022-05-10 03:19:50 +00:00

ShadowRylander commented 2022-05-10 03:20:01 +00:00

Author

Copy Link

Oops...

Oops...

ShadowRylander reopened this issue 2022-05-10 03:20:01 +00:00

witten commented 2022-05-10 03:23:43 +00:00

Owner

Copy Link

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?

ShadowRylander commented 2022-05-10 03:24:17 +00:00

Author

Copy Link

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.

ShadowRylander commented 2022-05-10 03:26:26 +00:00

Author

Copy Link

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.

ShadowRylander commented 2022-05-10 03:30:42 +00:00

Author

Copy Link

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...

witten commented 2022-05-10 03:33:27 +00:00

Owner

Copy Link

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.

ShadowRylander commented 2022-05-10 03:40:18 +00:00

Author

Copy Link

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 ```

witten commented 2022-05-10 04:15:47 +00:00

Owner

Copy Link

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.

ShadowRylander commented 2022-05-10 04:34:46 +00:00

Author

Copy Link

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

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

witten commented 2022-05-10 04:35:59 +00:00

Owner

Copy Link

Neither are mine. 😄

Neither are mine. 😄

❤️ 1

witten referenced this issue from a commit 2022-05-20 22:29:04 +00:00

When deep merging common configuration, merge colliding list values by appending them (#531).

witten commented 2022-05-20 22:33:50 +00:00

Owner

Copy Link

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 2022-05-20 22:33:50 +00:00

ShadowRylander commented 2022-05-23 16:15:18 +00:00

Author

Copy Link

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

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

👍 1

witten commented 2022-05-24 23:26:37 +00:00

Owner

Copy Link

Released in borgmatic 1.6.1!

Released in borgmatic 1.6.1!

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

spot-check

1.8.11

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

Subscribe

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#531

No description provided.