Sandro Bonazzola has posted comments on this change.

Change subject: Clean the code to pass pep8 checks
......................................................................


Patch Set 3:

(14 comments)

http://gerrit.ovirt.org/#/c/26314/3/src/__main__.py
File src/__main__.py:

Line 116:         logging.debug("_cmds(%s)" % _cmds)
Line 117:         proc = subprocess.Popen(_cmds,
Line 118:                    stdout=subprocess.PIPE,
Line 119:                    stderr=subprocess.PIPE
Line 120:         )
Indent:
    proc = subprocess.Popen(
        _cmds,
        stdout=subprocess.PIPE,
        stderr=subprocess.PIPE,
    )
Line 121:         stdout, stderr = proc.communicate()
Line 122:         returncode = proc.returncode
Line 123:         logging.debug("returncode(%s)" % returncode)
Line 124:         logging.debug("STDOUT(%s)" % stdout)


Line 273: 
Line 274:         # we want the items from the ImageUploader section only
Line 275:         try:
Line 276:             opts = ["--%s=%s" % (k, v)
Line 277:                        for k, v in cp.items("ImageUploader")]
Indent:
 opts = [
     "--%s=%s" % (k, v)
     for k, v in cp.items("ImageUploader")
 ]
Line 278:             (new_options, args) = self.parser.parse_args(args=opts,
Line 279:                                                          
values=self.options)
Line 280:             self.from_option_groups(new_options, self.parser)
Line 281:             self.from_options(new_options, self.parser)


Line 287:         if self.command not in Commands.ARY:
Line 288:             raise Exception(
Line 289:                 _(
Line 290:                 "%s is not a valid command.  Valid commands are '%s' 
or '%s'."
Line 291:                 ) % (
please use format
Line 292:                     self.command,
Line 293:                     Commands.LIST,
Line 294:                     Commands.UPLOAD
Line 295:                 )


Line 298:         if self.command == Commands.UPLOAD:
Line 299:             if len(args) <= 1:
Line 300:                 raise Exception(
Line 301:                     _("Files must be supplied "
Line 302:                       "for %s commands" % (Commands.UPLOAD)
please use format
Line 303:                     )
Line 304:                 )
Line 305:             for file in args[1:]:
Line 306:                 self.files.append(file)


Line 317:     # seem to behave the same way every time. Take a look at the link:
Line 318:     # http://stackoverflow.com/questions/4606942
Line 319:     def _prompt(self, prompt_function, key, msg=None):
Line 320:         value = get_from_prompt(
Line 321:             msg="Please provide the %s (CTRL+D to abort): " % msg,
please use format
Line 322:             prompter=prompt_function)
Line 323:         if value:
Line 324:             self[key] = value
Line 325:         else:


Line 540:                                 )
Line 541:                             else:
Line 542:                                 logging.debug(
Line 543:                                     "the storage domain didn't have a 
status "
Line 544:                                     "element.")
please go to new line with ')'
Line 545:                 else:
Line 546:                     logging.debug(
Line 547:                         _("DC(%s) does not have a storage domain.") % 
dcName)
Line 548:             if len(imageAry) > 0:


Line 547:                         _("DC(%s) does not have a storage domain.") % 
dcName)
Line 548:             if len(imageAry) > 0:
Line 549:                 imageAry.sort(key=get_name)
Line 550:                 fmt = "%-30s | %-25s | %s"
Line 551:                 print fmt % (
please use print(....) as in python3.
Line 552:                     _("Export Storage Domain Name"),
Line 553:                     _("Datacenter"),
Line 554:                     _("Export Domain Status")
Line 555:                 )


Line 574:             return
Line 575:         sd = self.api.storagedomains.get(exportdomain)
Line 576:         if sd is not None:
Line 577:             if sd.get_type() != 'export':
Line 578:                 raise Exception(_(
please go to new line with _(
Line 579:                     "The %s storage domain supplied is not of type 
'export'" %
Line 580:                     (exportdomain))
Line 581:                 )
Line 582:             id = sd.get_id()


Line 576:         if sd is not None:
Line 577:             if sd.get_type() != 'export':
Line 578:                 raise Exception(_(
Line 579:                     "The %s storage domain supplied is not of type 
'export'" %
Line 580:                     (exportdomain))
please use format and go to new line with last ')'
Line 581:                 )
Line 582:             id = sd.get_id()
Line 583:             storage = sd.get_storage()
Line 584:             if storage is not None:


Line 586:                 path = storage.get_path()
Line 587:             else:
Line 588:                 raise Exception(_(
Line 589:                     "A storage element was not found for the %s 
export domain"
Line 590:                     "." % exportdomain)
same here
Line 591:                 )
Line 592:             logging.debug('id=%s address=%s path=%s' % (id, address, 
path))
Line 593:             return (id, address, path)
Line 594:         else:


Line 593:             return (id, address, path)
Line 594:         else:
Line 595:             raise Exception(
Line 596:                 _("An export storage domain with a name of %s was not 
found." %
Line 597:                   exportdomain))
and here
Line 598: 
Line 599:     def unpack_ovf(self, ovf_file, dest_dir):
Line 600:         """
Line 601:         Given a path to an OVF .tgz this function will unpack it into


Line 610:             logging.error(
Line 611:                 _("Problem unpacking %s.  Message %s"
Line 612:                     % (ovf_file,
Line 613:                        str(e).strip())
Line 614:                 )
and here, well I think you got it...
Line 615:             )
Line 616:         finally:
Line 617:             tar.close()
Line 618:         return retVal


Line 1783:         "",
Line 1784:         "--cert-file",
Line 1785:         dest="cert_file",
Line 1786:         help="The CA certificate used to validate the engine."
Line 1787:            " (default=/etc/pki/ovirt-engine/ca.pem)",
missing () enclosing the string in help=(...)
Line 1788:         metavar="/etc/pki/ovirt-engine/ca.pem",
Line 1789:         default="/etc/pki/ovirt-engine/ca.pem"
Line 1790:     )
Line 1791: 


Line 1804:     parser.add_option("-f",
Line 1805:                       "--force",
Line 1806:                       dest="force",
Line 1807:                       help=_("replace like named files on the target 
file "
Line 1808:                              "server (default=off)"),
please go to new line after '(' and before ')'
Line 1809:                       action="store_true",
Line 1810:                       default=False)
Line 1811: 
Line 1812:     engine_group = OptionGroup(


-- 
To view, visit http://gerrit.ovirt.org/26314
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b37c4a5c2449a93a4bc8b90d74301b66ae945e3
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-image-uploader
Gerrit-Branch: master
Gerrit-Owner: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to