-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support multiple repositories (values separated by ;
)
#149
base: testing
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, only db data are copied twice in this case. YunoHost use mount --bind
to avoid to copy directory (that's why you dont' need to much space to backup your system).
With recent version of borg (and change in yunohost behavior), we could even imagine to pipe db export into borg command to avoid intermediary copy...)
In more, having 2 borg setup allows to trigger backup on 2 distinct date, which can be helpful.
@@ -5,13 +5,18 @@ app="${0#"./05-"}" | |||
app="${app%"_app"}" | |||
|
|||
BORG_PASSPHRASE="$(yunohost app setting $app passphrase)" | |||
repo="$(yunohost app setting $app repository)" #$4 | |||
repos="$(yunohost app setting $app repository)" #$4 | |||
if ssh-keygen -F "__SERVER__" >/dev/null ; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there are several repos, i guess we should adapt this part to. __SERVER__
is extracted from repository var if i remember well. And i think we should improve this part by deducing server from $repository (instead of hradcoding it during install script...)
;; | ||
backup) | ||
do_backup "$work_dir" "$name" "$repo" "$size" "$description" | ||
for repo in "${array_repos[@]}"; do | ||
do_backup "$work_dir" "$name" "$(sanitize_repo "$repo")" "$size" "$description" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep in mind if the first borg command to create the repo failed, the second won't be runned... (so it's not totally the same behavior than install twice the app)
This part should be adapted to : https://github.com/YunoHost-Apps/borg_ynh/blob/master/scripts/config#L50 |
And also the manifest.toml (testing branch) and the config_panel.toml |
@zamentur Right, thanks for the explanation, so knowing that, do you think it could be still a good idea? |
I guess it's still a good idea for apps which have a big db (for example synapse). |
Problem
Fixes #146
When backuping with multiple repositories, you'd have to install twice the borg app.
If I am not wrong, this causes to copy (link) apps data as many times as you have repositories, which can take time depending on the apps being backed up.
Solution
Instead, this patch proposes to install a single client for multiple repositories.
Note: Probably this PR would need #148, if we'd prefer separating values by newlines instead of semi-colons.
PR Status
Automatic tests
Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)