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.

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

Scheduling "retention" reads policy from "forget" #23

Closed
jkellerer opened this issue Dec 20, 2020 · 7 comments
Closed

Scheduling "retention" reads policy from "forget" #23

jkellerer opened this issue Dec 20, 2020 · 7 comments

Comments

@jkellerer
Copy link
Collaborator

jkellerer commented Dec 20, 2020

Finding

At the moment the following configuration schedules forget without any effect (at least with systemd):

default:
  retention:
    prune: true
    keep-last: 14
    schedule: '@daily'
    schedule-permission: 'system'

The reason is that retention isn't a command but a configuration section that is mapped to forget. The schedule invokes forget which reads the retention config from the forget config section (that is what I'd expect when forget is scheduled).
With the following workaround, it is possible to schedule forget (forget itself cannot be scheduled directly):

default:
  forget:
    prune: true
    keep-last: 14
  retention:
    schedule: '@daily'
    schedule-permission: 'system'

Suggestion

The idea to have retention policies defined with backup is great since it allows to have separate retention settings per tag & path. When called before or after backup and without prune (for large repos) it is also a fast operation. So this should not be changed, it is good as it is.

However, instead of artificially mapping retention to a scheduled forget command, I'd suggest to allow scheduling forget directly and remove the option to schedule retention (it is broken anyway).

See also #22 , when prune can be scheduled separately, then there are less use cases where scheduling forget (with or without prune) really makes sense, still I'd allow it as there may be cases where it is needed.

@jkellerer
Copy link
Collaborator Author

(I'm happy to provide a PR if this is accepted)

@creativeprojects
Copy link
Owner

hmmm yeah, I suppose being able to schedule forget makes sense, probably a bit more than retention which is supposed to be running along with a backup.

My intention was to keep only one configuration when running with a backup, or when running separately. But thinking about it, if you need to run it separately it means you probably need a different configuration anyway.

@jkellerer
Copy link
Collaborator Author

Yes, that’s also how I see it. Running along with backup is where retention makes perfectly sense. And in most cases that’s where it should run.

Main point for me is to run prune separately. But for the remaining cases where forget may need to be scheduled, it likely would need its own config as you also mentioned.

So instead of fixing the bug it might be better to adjust the logic as proposed above.

@creativeprojects
Copy link
Owner

So instead of fixing the bug it might be better to adjust the logic as proposed above.

I agree we can add a schedule of the forget command.
But I wouldn't want to break anybody's configuration relying on the retention schedule: we could fix the issue, or at a minimum I would mark it deprecated and then remove it later.

@jkellerer
Copy link
Collaborator Author

Yes that makes sense

@jkellerer
Copy link
Collaborator Author

Hi @creativeprojects , a PR that adds support to schedule forget was just added.

Since fixing the reported problem would break setups of existing users, I'd deprecate scheduling retention in favour of scheduling forget instead, and document that scheduling retention takes retention settings from the forget section. The deprecation notice could simply instruct to copy schedule settings into the forget section and behaviour should remain the same but is then consistent with the profile settings.

@jkellerer
Copy link
Collaborator Author

0.11.0 includes all changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants