Alon Bar-Lev has posted comments on this change. Change subject: packaging: setup: support compressed backup/restore ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/26073/3/packaging/setup/ovirt_engine_setup/database.py File packaging/setup/ovirt_engine_setup/database.py: Line 650: def backup( Line 651: self, Line 652: dir, Line 653: prefix, Line 654: compr='bzip2', compr is not clear, usually filter is better. this also means you need to add dependency of bzip2 to setup, but better default to plain and compress only if user chooses, this way he will also provide the suffix, filter == "cat". I also think you should provide the entire command line as tuple. Line 655: compr_suffix='.bz2', Line 656: ): Line 657: fd, backupFile = tempfile.mkstemp( Line 658: prefix='%s-%s.' % ( Line 701: stdin=dump_proc.stdout, Line 702: stdout=output_file, Line 703: stderr=subprocess.PIPE, Line 704: close_fds=True, Line 705: ) another option is just to use shell pipe and specify shell=True, the problem with this approach is the need to escape everything. however, I would recommend to add a function of left and right, and use it in both cases. def execFilter(left, right, envAppend=None): Line 706: dump_stdout, dump_stderr = dump_proc.communicate() Line 707: compr_stdout, compr_stderr = compr_proc.communicate() Line 708: dump_rc = dump_proc.returncode Line 709: compr_rc = compr_proc.returncode Line 732: backupFile, Line 733: compr='bzip2', Line 734: compr_suffix='.bz2', Line 735: ): Line 736: compressed = backupFile.endswith(compr_suffix) please do not try to guess, either you get the filter or not, in any case you will get entire file name and not suffix. Line 737: with open(backupFile, 'r') as input_file: Line 738: env = os.environ.copy() Line 739: env.update({ Line 740: 'PGPASSWORD': '', Line 746: else: Line 747: decompr_proc = subprocess.Popen( Line 748: args=( Line 749: compr, Line 750: '-d', I am unsure this is valid assumption, parameter should be the entire command-line. Line 751: ), Line 752: stdin=input_file, Line 753: stdout=subprocess.PIPE, Line 754: stderr=subprocess.PIPE, -- To view, visit http://gerrit.ovirt.org/26073 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I684f025c9c1d29c701068ba967be5a2115c7a6fb Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
