Project

General

Profile

Bug #8196

rsync actually syncs files in --test mode

Added by shred about 3 years ago. Updated about 3 years ago.

Status:
Fix committed
Priority:
Normal
Assignee:
-
Category:
rsync handler
Target version:
Start date:
11/02/2014
Due date:
% Done:

100%

QA Check:
Pass

Description

When backupninja is started with --test option, rsync action does still actually sync files!

rsync should be invoked with --dry-run option when backupninja runs in --test mode.

rsync.patch View (379 Bytes) shred, 11/04/2014 11:30 PM

rsync.patch View (570 Bytes) shred, 11/06/2014 06:44 PM

History

#1 Updated by shred about 3 years ago

This patch worked for me.

#2 Updated by rhatto about 3 years ago

  • Assignee changed from rhatto to intrigeri
  • QA Check set to Pass

Thanks for the patch :)

I think it's ready for merging. Shall I do it?

#3 Updated by rhatto about 3 years ago

Patch available as commit c649339ba41b4bc748598ca13c4363e25f6eba1f (branch bug/8196).

#4 Updated by intrigeri about 3 years ago

  • Status changed from New to Confirmed
  • Assignee changed from intrigeri to rhatto
  • % Done changed from 0 to 50
  • QA Check changed from Pass to Info Needed

rhatto, did you test the proposed commit on top of our current master branch? If so, I'll merge your branch.

#5 Updated by rhatto about 3 years ago

  • Assignee changed from rhatto to shred
  • QA Check changed from Info Needed to Dev Needed

Yes, I tested with ninjahelper and --dry-run is passed to rsync when the action is invoke in test mode. But I noted that the patch doesn't fix everything and metadata can still be rotated when the long format is used.

shred, could you provide a patch having the same check on the prepare_storage function? In that case we would not be testing if the action can create remote folders.

Another option would be to change rotate_* and setup_long_dirs_* functions to just try to create a dummy/test file/folder in the remote destination.

#6 Updated by shred about 3 years ago

Yes, you're right...

The attached patch will return from the prepare_storage function in test mode, so rotation will not be tested.

I'm sorry that I can only provide a patch for the easier solution, but I'm not very fluent with shell scripting. (I'd rather prefer Python... ;-) )

#7 Updated by intrigeri about 3 years ago

  • Assignee changed from shred to rhatto
  • QA Check changed from Dev Needed to Ready for QA

#8 Updated by rhatto about 3 years ago

  • Assignee changed from rhatto to intrigeri
  • QA Check changed from Ready for QA to Pass

It worked but I had to make another change.

Then the test suceeded. I'm fine with having the handler not trying rotation in the test mode, so I think it's ready to merge and close this bug.

#9 Updated by intrigeri about 3 years ago

  • Status changed from Confirmed to Fix committed
  • Assignee deleted (intrigeri)
  • Target version set to 1.0.2
  • % Done changed from 50 to 100

Merged, thanks!

Also available in: Atom PDF