[node-patches] Change in ovirt-node[master]: edit-node: pep8 spacing and line length fixes

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: edit-node: pep8 spacing and line length fixes
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(10 inline comments)

....................................................
File tools/edit-node
Line 156:         try:
Line 157:             self._ImageCreator__builddir = tempfile.mkdtemp(
Line 158:                 dir=os.path.abspath(self.tmpdir),
Line 159:                 prefix="edit-liveos-"
Line 160:             )
Is this identation correct according to pep8? (I wonder about the closing bracket)
Line 161:         except OSError, (err, msg):
Line 162:             raise CreatorError("Failed create build directory in %s: %s" %
Line 163:                                (self.tmpdir, msg))
Line 164: 


Line 177:         try:
Line 178:             subprocess.call([script], preexec_fn=self._chroot)
Line 179:         except OSError, e:
Line 180:             raise CreatorError("Failed to execute script %s, %s " %
Line 181:                               (script, e))
one space is missing before the opening bracket in the second line).
Line 182:         finally:
Line 183:             os.unlink(path)
Line 184: 
Line 185:     def mount(self, base_on, cachedir=None):


Line 243:                           ("/dev/pts", None), ("/dev/shm", None),
Line 244:                           (cachesrc, "/var/cache/yum")]:
Line 245:             self._ImageCreator__bindmounts.append(BindChrootMount(
Line 246:                 f, self._instroot, dest)
Line 247:             )
same as two above
Line 248: 
Line 249:         self._do_bindmounts()
Line 250:         self.__copy_img_root(base_on)
Line 251:         self._brand(self._builder)


Line 378:                           ("/dev/pts", None), ("/dev/shm", None),
Line 379:                           (cachesrc, "/var/cache/yum")]:
Line 380:             self._ImageCreator__bindmounts.append(BindChrootMount(
Line 381:                 f, self._instroot, dest)
Line 382:             )
same questions as in my last comment above
Line 383: 
Line 384:         self._do_bindmounts()
Line 385:         self.__copy_img_root(base_on)
Line 386:         self._brand(self._builder)


Line 525:         if self.ks:
Line 526:             # bootloader --append "!opt-to-remove opt-to-add"
Line 527:             for param in kickstart.get_kernel_args(self.ks, "").split():
Line 528:                 if param.startswith('!'):
Line 529:                     param = param[1:]
Ha! gotcha! that's not a line length fix ;)
Line 530:                     # remove parameter prefixed with !
Line 531:                     args.append('-e')
Line 532:                     args.append("/^  append/s/%s //" % param)
Line 533:                     # special case for last parameter


Line 608:                 print "Installing %s\r" % (hdr["name"])
Line 609:                 fd = os.open(path, os.O_RDONLY)
Line 610:                 nvr = '%s-%s-%s' % (hdr['name'], hdr['version'],
Line 611:                       hdr['release']
Line 612:                 )
same as somewhere above
Line 613:                 self.fdnos[nvr] = fd
Line 614:                 return fd
Line 615: 
Line 616:             elif what == rpm.RPMCALLBACK_INST_CLOSE_FILE:


Line 616:             elif what == rpm.RPMCALLBACK_INST_CLOSE_FILE:
Line 617:                 hdr, path = mydata
Line 618:                 nvr = '%s-%s-%s' % (hdr['name'], hdr['version'],
Line 619:                       hdr['release']
Line 620:                 )
same as somewhere above
Line 621:                 os.close(self.fdnos[nvr])
Line 622: 
Line 623:             elif what == rpm.RPMCALLBACK_INST_PROGRESS:
Line 624:                 hdr, path = mydata


Line 865:             output, err = f.communicate()
Line 866:         print "Copying Manifests"
Line 867:         for file in glob.glob("%s/*manifest-*" % self._instroot):
Line 868:             rootfs_manifest_dir = "%s/etc/ovirt-plugins-manifests.d" % \
Line 869:                                   self._instroot
Is this identation correct?
you normally only align on a char-in-previous-line base if there are brackets aroudn the expression, e.g.:

  some_long_val = ("bar %s" %
                   baz) # aligned to first char in bracket above

otherwise

  some_long_val = "bar %s" % \
      baz  # 4 spaces
Line 870:             os.system("mkdir -p %s" % rootfs_manifest_dir)
Line 871:             cmd = "cp %s %s" % (file, rootfs_manifest_dir)
Line 872:             f = subprocess.Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT)
Line 873:             if f.returncode > 0:


Line 1235:             os.system("mkdir %s/tmp/yumrepo" % self._instroot)
Line 1236:             os.system("touch %s/tmp/yumrepo/%s" % (self._instroot,
Line 1237:                       os.path.basename(pkgs)))
Line 1238:             os.system("mount -o bind %s %s/tmp/yumrepo/%s" %
Line 1239:                      (pkgs, self._instroot, os.path.basename(pkgs)))
here is one space missing before the opening bracket
Line 1240:             print os.listdir("%s/tmp/yumrepo" % self._instroot)
Line 1241:             yum_cmd = "yum localinstall -y /tmp/yumrepo/%s" % \
Line 1242:                       os.path.basename(pkgs)
Line 1243:         else:


Line 1238:             os.system("mount -o bind %s %s/tmp/yumrepo/%s" %
Line 1239:                      (pkgs, self._instroot, os.path.basename(pkgs)))
Line 1240:             print os.listdir("%s/tmp/yumrepo" % self._instroot)
Line 1241:             yum_cmd = "yum localinstall -y /tmp/yumrepo/%s" % \
Line 1242:                       os.path.basename(pkgs)
i suppose this is over-indented ( to many whitespaces)
Line 1243:         else:
Line 1244:             yum_cmd = "yum install -y %s " % pkgs
Line 1245:         # make sure rpm keys are imported
Line 1246:         rpm_cmd = "rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY*"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe6dad850e2dacdcd9e957176c96a93ea691295
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>