Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Unable to use some placeholders in archive names #1954

Open
2 tasks done
daPhipz opened this issue Mar 2, 2024 · 8 comments · May be fixed by #1956
Open
2 tasks done

Unable to use some placeholders in archive names #1954

daPhipz opened this issue Mar 2, 2024 · 8 comments · May be fixed by #1956
Labels
good first issue Simple change to start learning code base help wanted This issue is available, comment if you want to fix it

Comments

@daPhipz
Copy link

daPhipz commented Mar 2, 2024 •
edited

Description

When trying to add the following placeholders (see here) to the archive name, Vorta says "Error in archive name template.":

  • {reverse-fqdn}
  • {utcnow}
  • {pid}
  • {borgversion}
  • {borgmajor}
  • {borgminor}
  • {borgpatch}

When I navigate away from that tab in Vorta (e.g. to the "Schedule" tab) and then go back, the "problematic" part of the archive name is removed from the text input field

Reproduction

  • I tried to reproduce the issue.
  • I was able to reproduce the issue.

OS

Kubuntu 22.10

Version of Vorta

0.9.1

What did you install Vorta with?

Flatpak

Version of Borg

1.2.4 according to Vorta, 1.2.6 according to borg --version (I guess one comes with the Flatpak and the other is a deb package)

Logs

No response
@m3nu
Copy link
Contributor

m3nu commented Mar 4, 2024

We may not pass those to Borg and our current placeholders are filled by Vorta only. I think. If this is the case, we should look into passing them through, as you expect here.

@m3nu m3nu added good first issue Simple change to start learning code base help wanted This issue is available, comment if you want to fix it labels Mar 4, 2024
@AdwaitSalankar
Copy link
Contributor

AdwaitSalankar commented Mar 6, 2024 •
edited

@m3nu I think that if we pass them through to borg, we may not be able to show the preview.
We can just add the missing keys from borg to vorta.

https://github.com/borgbackup/borg/blob/b82bf4a232c6c7d63796d2366a147f74c5ab13dc/src/borg/helpers/parseformat.py#L264C5-L272C20

@AdwaitSalankar AdwaitSalankar linked a pull request Mar 6, 2024 that will close this issue
Draft
9 tasks
@m3nu
Copy link
Contributor

m3nu commented Mar 6, 2024

Will this change solve it for you, @daPhipz ? It's a clever change actually.

@daPhipz
Copy link
Author

daPhipz commented Mar 6, 2024 •
edited

@m3nu I think that if we pass them through to borg, we may not be able to show the preview.
We can just add the missing keys from borg to vorta.

https://github.com/borgbackup/borg/blob/b82bf4a232c6c7d63796d2366a147f74c5ab13dc/src/borg/helpers/parseformat.py#L264C5-L272C20

I don't really know how to (read that) code - would this solution mean that the placeholders would work, but the preview text under the text field could be wrong? Or where is the compromise?

I personally don't care about the preview if the actual naming works, but maybe the user should be informed in that case.

@AdwaitSalankar
Copy link
Contributor

Both (Placeholders and the Preview) will work here. The naming would be same as preview.

@daPhipz
Copy link
Author

daPhipz commented Mar 7, 2024

In that case, I am perfectly fine with this solution! Thank you very much for this quick PR! 😍

@AdwaitSalankar
Copy link
Contributor

@m3nu, is this PR good to be closed then?

@m3nu
Copy link
Contributor

m3nu commented Mar 14, 2024

Yes, we will merge it. Let's just give others a chance to look at it too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Simple change to start learning code base help wanted This issue is available, comment if you want to fix it
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add new placeholder keys AdwaitSalankar/vorta
3 participants
@m3nu @daPhipz @AdwaitSalankar