On 01/12/2015 02:28 PM, Jan Cholasta wrote:
the attached patch fixes <https://fedorahosted.org/freeipa/ticket/4797>.
Note that --data with data-only backup and --logs-only with data-only
restore are deliberately ignored and considered no-op.
1. I'm not sure how relative path to backup dir should work:
I have a backup: /var/lib/ipa/backup/ipa-data-2015-01-13-10-53-06/
Absolute path works great. But:
fails with `ipa-restore: error: must provide path to backup directory`
Ugly hybrid (I was in home dir):
I.e., dir name won't pass
if not os.path.isdir(self.args):
and the second concatenation is weird.
2. Data backup cannot be done because the first 'for' never breaks
+ for instance in instances:
+ for backend in backends:
+ db_dir = (paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
+ (instance, backend))
+ if os.path.exists(db_dir):
+ raise admintool.ScriptError(
+ "Cannot restore a data backup into an empty
3. When #2 fixed, data backup with --no-logs doesn't raise warning as
requested in ticket.
4. Since backup type is detected automatically(--data doesn't have to be
specified for data restore), I guess that the ticket requirement: `Here
expecting an error message that --online option can only provided along
with --data option. ` is invalid, right?
Man page states that --online requires --data option, which is not true.
5. Nitpick: imho option validation should be done before temp dir creation.
6. pep8, fixing them is up to you:
./ipaserver/install/ipa_restore.py:150:5: E129 visually indented line
with same indent as next logical line
./ipaserver/install/ipa_restore.py:184:13: E128 continuation line
under-indented for visual indent
Freeipa-devel mailing list