ungleich-public/ccollect
6
0
Fork 1
Code Issues Pull requests 1 Projects Releases Wiki Activity

Allow interval per source to be overwritten to 0 to skip the source in this interval. #2

Open
skybeam wants to merge 3 commits from skybeam/ccollect:master into master
pull from: skybeam/ccollect:master
merge into: ungleich-public:master
Conversation 4 Commits 3 Files changed 2 +19 -2
skybeam commented 2023-01-14 21:59:34 +00:00
First-time contributor
Copy link

This change allows to overwrite an interval to 0 for a specific source in order to skip it for this interval.
This is useful if you have default intervals configured and you would like to skip certain sources for specific intervals.

This change allows to overwrite an interval to 0 for a specific source in order to skip it for this interval. This is useful if you have default intervals configured and you would like to skip certain sources for specific intervals.
skybeam added 1 commit 2023-01-14 21:59:36 +00:00 
This change allows to overwrite an interval to 0 for a specific source in order to skip it for this interval.
This is useful if you have default intervals configured and you would like to skip certain sources for specific intervals.
nico reviewed 2023-01-15 21:14:52 +00:00
ccollect
@ -766,3 +766,3 @@
_exit_err "Listing oldest backup failed")
_techo "Using ${oldest_bak} for destination dir ${destination_dir}"
if mv "${oldest_bak}" "${destination_dir}"; then
if mv "${oldest_bak}" "${destination_dir}" 2>/dev/null; then
nico commented 2023-01-15 21:14:52 +00:00
Owner
Copy link

Why skip potential error messages?

Why skip potential error messages?
skybeam commented 2023-01-15 21:17:41 +00:00
Author
First-time contributor
Copy link

Because oldest_bak can be empty and this causing mv to spit an error - which is OK as we are only interested in the return code telling us if the move was successful. In this case the failed move is expected. No need for an error message.

Because oldest_bak can be empty and this causing mv to spit an error - which is OK as we are only interested in the return code telling us if the move was successful. In this case the failed move is expected. No need for an error message.
nico reviewed 2023-01-15 21:15:39 +00:00
ccollect Outdated
@ -780,3 +780,3 @@
_techo "Removing ${remove} backup(s)..."
if [ -z "${ls_rm_exclude}" ]; then
if [ -z "${ls_rm_exclude}" -o ${c_interval} -le 0 ]; then
nico commented 2023-01-15 21:15:39 +00:00
Owner
Copy link

If we want to match 0, we should do -eq 0, otherwise the documentation will need to be adjusted.

If we want to match 0, we should do `-eq 0`, otherwise the documentation will need to be adjusted.
skybeam commented 2023-01-15 21:18:31 +00:00
Author
First-time contributor
Copy link

Fine with me, usually errors are positive integers and I learned never ever to match numbers with "eq" - but for integers here it might be ok, so feel free to use -eq.

Fine with me, usually errors are positive integers and I learned never ever to match numbers with "eq" - but for integers here it might be ok, so feel free to use -eq.
nico reviewed 2023-01-15 21:15:47 +00:00
ccollect Outdated
@ -796,0 +796,4 @@
#
# Skip backup of this source if interval is zero.
#
if [ ${c_interval} -le 0 ]; then
nico commented 2023-01-15 21:15:47 +00:00
Owner
Copy link

Same here

Same here
nico commented 2023-01-15 21:16:26 +00:00
Owner
Copy link

I think overall the idea is not bad, I'd recommend to go with -eq 0 and we would also need to document the behaviour, not only implement it.

I think overall the idea is not bad, I'd recommend to go with `-eq 0` and we would also need to document the behaviour, not only implement it.
skybeam commented 2023-01-15 21:20:52 +00:00
Author
First-time contributor
Copy link

I did have a very quick look at the code only. Not sure if it is not ideal at all, but I am definitely missing an option to disable an interval for certain sources.

I typically have daily, weekly, monthly and yearly intervals, but for some sources I don't want to have daily as weekly is far sufficient. So there would be only the option to not define the intervals in the defaults but this leaving me to require defining the intervals in all other sources. Would be much easier to be able to override an interval to 0 to disable it for a certain source.

I did have a very quick look at the code only. Not sure if it is not ideal at all, but I am definitely missing an option to disable an interval for certain sources. I typically have daily, weekly, monthly and yearly intervals, but for some sources I don't want to have daily as weekly is far sufficient. So there would be only the option to not define the intervals in the defaults but this leaving me to require defining the intervals in all other sources. Would be much easier to be able to override an interval to 0 to disable it for a certain source.
nico commented 2023-01-15 21:40:33 +00:00
Owner
Copy link

I think overall the idea is not bad, I'd recommend to go with -eq 0 and we would also need to document the behaviour, not only implement it.

I think overall the idea is not bad, I'd recommend to go with `-eq 0` and we would also need to document the behaviour, not only implement it.
skybeam added 1 commit 2023-01-15 22:31:04 +00:00
skybeam added 1 commit 2023-01-15 23:32:29 +00:00 
Update documentation to specify that intervals with value 0 are valid to skip an interval explicitly for a given source.
skybeam commented 2023-01-15 23:33:02 +00:00
Author
First-time contributor
Copy link

Did some update to the documentation as well.

Did some update to the documentation as well.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u master:skybeam-master
git checkout skybeam-master

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout master
git merge --no-ff skybeam-master
git checkout skybeam-master
git rebase master
git checkout master
git merge --ff-only skybeam-master
git checkout skybeam-master
git rebase master
git checkout master
git merge --no-ff skybeam-master
git checkout master
git merge --squash skybeam-master
git checkout master
git merge --ff-only skybeam-master
git checkout master
git merge skybeam-master
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 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: ungleich-public/ccollect#2
No description provided.
Delete branch "skybeam/ccollect:master"

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?