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

[feature request] label/tag a repo #635

New Issue
Open
opened 2023-02-06 20:32:35 +00:00 by eoli3n · 11 comments
eoli3n commented 2023-02-06 20:32:35 +00:00
Copy Link

What I'm trying to do and why

Label a repo, to be able to use --repository label1 instead of the full path copypasted from borgmatic list. Useful when using multiple config files with /etc/borgmatic.d, or multiple repositories in the same file.

Syntax could be

location:
  repositories:
    - path: borgbase.com/stuff
      label: borgbase
    - path: rsync.net/stuff
      label: rsyncnet

Problem is that it uses a dict instead of the current list, and it breaks compatibility.

If a label is set, it could be used in borgmatic list --short

$ borgmatic list --short
borgbase: Listing archives
host1-2022-10-02T11:34:27
rsyncnet: Listing archives
host1-2022-10-02T11:34:27

Mount a backup

borgmatic mount --archive host1-2022-10-02T11:34:27 --repository borgbase --mount-point /mnt
#### What I'm trying to do and why Label a repo, to be able to use ``--repository label1`` instead of the full path copypasted from ``borgmatic list``. Useful when using multiple config files with ``/etc/borgmatic.d``, or multiple repositories in the same file. Syntax could be ``` location: repositories: - path: borgbase.com/stuff label: borgbase - path: rsync.net/stuff label: rsyncnet ``` Problem is that it uses a dict instead of the current list, and it breaks compatibility. If a label is set, it could be used in ``borgmatic list --short`` ``` $ borgmatic list --short borgbase: Listing archives host1-2022-10-02T11:34:27 rsyncnet: Listing archives host1-2022-10-02T11:34:27 ``` Mount a backup ``` borgmatic mount --archive host1-2022-10-02T11:34:27 --repository borgbase --mount-point /mnt ```
witten commented 2023-02-06 21:12:42 +00:00
Owner
Copy Link

This is a great idea! I think the string vs. dict thing could be sidestepped by auto-converting any list of strings into a list of dicts internally.. There is precedent for doing that with other configuration options in borgmatic.

This is a great idea! I think the string vs. dict thing could be sidestepped by auto-converting any list of strings into a list of dicts internally.. There is precedent for doing that with other configuration options in borgmatic.
witten added the
design finalized
good first issue
 labels 2023-02-06 21:12:56 +00:00
diivi commented 2023-03-07 18:22:42 +00:00
Collaborator
Copy Link

This is a problem that I face too, I would like to work on the solution!
Will explore the code a little, anything you have to sugget @witten ?

This is a problem that I face too, I would like to work on the solution! Will explore the code a little, anything you have to sugget @witten ?
witten commented 2023-03-07 19:29:21 +00:00
Owner
Copy Link

Sure! The auto-converting of lists to dicts internally I mentioned in the last comment could be done in the existing borgmatic/config/normalize.py:normalize(). That way, "old-style" configuration files would continue to work. (Which suggests repository labels should be optional.) There are also a number of places throughout the borgmatic source that expect repositories to be a list of strings instead of list of dicts, so those would have to be updated as well.

Additionally, the code that resolves repository names (matching a repository specified on the command-line to a repository specified in configuration) is at borgmatic/config/validate.py:repositories_match(). That could be enhanced to support matching repositories by label and not just path, although the relevant configuration would have to get passed in for that to work.

Lastly, code that looks like this throughout the borgmatic source would probably need updating to use repository labels (if set):

logger.answer(f'{repository}: Listing archive {archive}')
Sure! The auto-converting of lists to dicts internally I mentioned in the last comment could be done in the existing `borgmatic/config/normalize.py:normalize()`. That way, "old-style" configuration files would continue to work. (Which suggests repository labels should be optional.) There are also a number of places throughout the borgmatic source that expect `repositories` to be a list of strings instead of list of dicts, so those would have to be updated as well. Additionally, the code that resolves repository names (matching a repository specified on the command-line to a repository specified in configuration) is at `borgmatic/config/validate.py:repositories_match()`. That could be enhanced to support matching repositories by label and not just path, although the relevant configuration would have to get passed in for that to work. Lastly, code that looks like this throughout the borgmatic source would probably need updating to use repository labels (if set): ```python logger.answer(f'{repository}: Listing archive {archive}') ```
👀 1
diivi commented 2023-03-07 21:23:24 +00:00
Collaborator
Copy Link

That way, "old-style" configuration files would continue to work. (Which suggests repository labels should be optional.)

I checked the current schema.yaml and did this:

repositories:
                type: array
                items:
                    type: object
                    required:
                        - path
                    additionalProperties: false
                    properties:
                        path:
                            type: string
                            description: |
                                Path to local or remote repository (required). 
                                are expanded. Multiple repositories are backed up to
                                in sequence. Borg placeholders can be used. See the
                                output of "borg help placeholders" for details. See
                                ssh_command for SSH options like identity file or
                                port. If systemd service is used, then add local
                                repository paths in the systemd service file to the
                                ReadWritePaths list.                                
                            example:
                                - ssh://user@backupserver/./sourcehostname.borg
                                - ssh://user@backupserver/./{fqdn}
                                - /var/local/backups/local.borg
                        label:
                            type: string
                            description: |
                                Optional label for the repository.                                
                            example: backupserver

But this expects the repositories to be an array of objects instead of an array of strings.
I couldn't understand your discussion about "lists to dicts", here I should be getting a list of dicts like this right?

[{
	path: "x/y/z",
    	label: "some repo
}]

What should I convert this data structure to? Assuming earlier it used to be

["x/y/z"]

If the label does not need to be handled in normalize.py, I can convert the list of dicts(1) back to the list of strings(2), effectively ignoring it here and only handling it in the places where you mentioned matching/handling of a repository name takes place

> That way, "old-style" configuration files would continue to work. (Which suggests repository labels should be optional.) I checked the current `schema.yaml` and did this: ```yaml repositories: type: array items: type: object required: - path additionalProperties: false properties: path: type: string description: | Path to local or remote repository (required). are expanded. Multiple repositories are backed up to in sequence. Borg placeholders can be used. See the output of "borg help placeholders" for details. See ssh_command for SSH options like identity file or port. If systemd service is used, then add local repository paths in the systemd service file to the ReadWritePaths list. example: - ssh://user@backupserver/./sourcehostname.borg - ssh://user@backupserver/./{fqdn} - /var/local/backups/local.borg label: type: string description: | Optional label for the repository. example: backupserver ``` But this expects the repositories to be an array of objects instead of an array of strings. I couldn't understand your discussion about "lists to dicts", here I should be getting a list of dicts like this right? ```python [{ path: "x/y/z", label: "some repo }] ``` What should I convert this data structure to? Assuming earlier it used to be ```python ["x/y/z"] ``` If the label does not need to be handled in `normalize.py`, I can convert the list of dicts(1) back to the list of strings(2), effectively ignoring it here and only handling it in the places where you mentioned matching/handling of a repository name takes place
witten commented 2023-03-08 00:12:17 +00:00
Owner
Copy Link

Your new schema and resulting list of dicts looks completely correct to me. I'm assuming that will be the new standard data structure for repositories going forward. So what I meant by converting "lists to dicts" is that if the old data structure, e.g. ["x/y/z"] (a list of strings), happens to be present in a configuration file, it needs to get converted to the new data structure (a list of dicts) before the schema is applied. That way all the borgmatic code can get updated to expect the new structure and can forget all about the old structure (outside of normalize.py).

To summarize, the general order of events is:

  1. Load configuration file into a data structure.
  2. Normalize it with normalize.py.
  3. Validate the normalized data structure with the configuration schema.
  4. Actually use the data structure!

This is all already in place today, and my suggested change is just to update normalize.py. And also of course the schema changes you've already made!

Please let me know if any of this doesn't make sense.

Your new schema and resulting list of dicts looks completely correct to me. I'm assuming that will be the new standard data structure for `repositories` going forward. So what I meant by converting "lists to dicts" is that if the *old* data structure, e.g. `["x/y/z"]` (a list of strings), happens to be present in a configuration file, it needs to get converted to the new data structure (a list of dicts) before the schema is applied. That way all the borgmatic code can get updated to expect the new structure and can forget all about the old structure (outside of `normalize.py`). To summarize, the general order of events is: 1. Load configuration file into a data structure. 2. Normalize it with `normalize.py`. 3. Validate the normalized data structure with the configuration schema. 4. Actually use the data structure! This is all already in place today, and my suggested change is just to update `normalize.py`. And also of course the schema changes you've already made! Please let me know if any of this doesn't make sense.
👍 1
witten commented 2023-03-27 23:21:40 +00:00
Owner
Copy Link

Released in borgmatic 1.7.10! Thanks to @diivi for doing this work!

Released in borgmatic 1.7.10! Thanks to @diivi for doing this work!
🎉 2
eoli3n commented 2023-05-15 19:03:21 +00:00
Author
Copy Link

I just tested this, and notice that a little part is missing.
Maybe not important, but --short doesn't use the label, it still uses the full path.

➜ sudo borgmatic list --short --repository vps
ssh://vps/data/zfs/backups/osz: Listing archives
osz-2023-03-31T09:43:25
osz-2023-04-02T09:18:42
osz-2023-04-07T20:56:28

But It should return

➜ sudo borgmatic list --short --repository vps
vps: Listing archives
osz-2023-03-31T09:43:25
osz-2023-04-02T09:18:42
osz-2023-04-07T20:56:28

That's the cherry at the top of that feature :P

I just tested this, and notice that a little part is missing. Maybe not important, but ``--short`` doesn't use the label, it still uses the full path. ``` ➜ sudo borgmatic list --short --repository vps ssh://vps/data/zfs/backups/osz: Listing archives osz-2023-03-31T09:43:25 osz-2023-04-02T09:18:42 osz-2023-04-07T20:56:28 ``` But It should return ``` ➜ sudo borgmatic list --short --repository vps vps: Listing archives osz-2023-03-31T09:43:25 osz-2023-04-02T09:18:42 osz-2023-04-07T20:56:28 ``` That's the cherry at the top of that feature :P
👍 1
eoli3n reopened this issue 2023-05-15 19:03:21 +00:00
witten commented 2023-05-15 19:11:39 +00:00
Owner
Copy Link

Makes sense! Thanks for bringing this to our attention.

Makes sense! Thanks for bringing this to our attention.
witten commented 2023-05-16 16:39:01 +00:00
Owner
Copy Link

I did part of this for some of the high-level functions, but more work will be needed to change the log statements for the remainder of the code base.

I did part of this for some of the high-level functions, but more work will be needed to change the log statements for the remainder of the code base.
❤️ 1
witten removed the
good first issue
 label 2023-06-28 18:39:36 +00:00
diivi was assigned by witten 2023-08-24 21:57:46 +00:00
diivi commented 2023-12-15 17:04:05 +00:00
Collaborator
Copy Link

hey @witten, did you end up solving this? Sorry for the late response, my exam just ended.

Looks like

repository.get("label", repository["path"]) throughout the codebase is done to implement this?

hey @witten, did you end up solving this? Sorry for the late response, my exam just ended. Looks like `repository.get("label", repository["path"])` throughout the codebase is done to implement this?
witten commented 2023-12-15 17:53:02 +00:00
Owner
Copy Link

Hey @diivi! Glad to hear you've gotten through your exams. You are correct, what you propose is most of the work remaining on this ticket. The other 90% though is passing through the full repository dict to everywhere in the code that needs it for this!

Hey @diivi! Glad to hear you've gotten through your exams. You are correct, what you propose is most of the work remaining on this ticket. The other 90% though is passing through the full `repository` dict to everywhere in the code that needs it for this!
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
diivi
3 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#635
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?