Change in vdsm[master]: open: Change file() to open()

Dima Kuznetsov has uploaded a new change for review.

Change subject: open: Change file() to open()
......................................................................

open: Change file() to open()

Update file calls to open calls due to file's deprecation.

Change-Id: I80007044be1c648ebb668704731eae8b83366025
Signed-off-by: Dima Kuznetsov 
---
M vdsm/API.py
M vdsm/hooks.py
M vdsm/storage/fileUtils.py
M vdsm/storage/hba.py
M vdsm/virt/sampling.py
M vdsm_hooks/nestedvt/before_vm_start.py
6 files changed, 13 insertions(+), 13 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/26776/1

diff --git a/vdsm/API.py b/vdsm/API.py
index 94b39b6..6724559 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -185,8 +185,8 @@
                     # parmas were stored.
                     fname = self._cif.prepareVolumePath(paramFilespec)
                     try:
-                        with file(fname) as f:
-                            pickledMachineParams = pickle.load(f)
+                        with open(fname) as file:
+                            pickledMachineParams = pickle.load(file)

if type(pickledMachineParams) == dict:
                             self.log.debug('loaded pickledMachineParams ' +
diff --git a/vdsm/hooks.py b/vdsm/hooks.py
index 75f33f9..ba7bad7 100644
--- a/vdsm/hooks.py
+++ b/vdsm/hooks.py
@@ -341,8 +341,8 @@

def _getScriptInfo(path):
     try:
-        with file(path) as f:
-            md5 = hashlib.md5(f.read()).hexdigest()
+        with open(path) as file:
+            md5 = hashlib.md5(file.read()).hexdigest()
     except:
         md5 = ''
     return {'md5': md5}
diff --git a/vdsm/storage/fileUtils.py b/vdsm/storage/fileUtils.py
index 56dc1ef..4ffe113 100644
--- a/vdsm/storage/fileUtils.py
+++ b/vdsm/storage/fileUtils.py
@@ -401,6 +401,6 @@

def padToBlockSize(path):
-    with file(path, 'a') as f:
-        size = os.fstat(f.fileno()).st_size
-        os.ftruncate(f.fileno(), 512 * ((size + 511) / 512))
+    with open(path, 'a') as file:
+        size = os.fstat(file.fileno()).st_size
+        os.ftruncate(file.fileno(), 512 * ((size + 511) / 512))
diff --git a/vdsm/storage/hba.py b/vdsm/storage/hba.py
index da3feef..b649f6c 100644
--- a/vdsm/storage/hba.py
+++ b/vdsm/storage/hba.py
@@ -43,8 +43,8 @@
     """
     hbas = []
     try:
-        with file(ISCSI_INITIATOR_NAME) as f:
-            for line in f:
+        with open(ISCSI_INITIATOR_NAME) as file:
+            for line in file:
                 if line.startswith(INITIATOR_NAME):
                     hba = {'InitiatorName': line.split("=")[1].strip()}
                     hbas.append(hba)
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index 991c969..ed8f58e 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -189,8 +189,8 @@
             self.cpuLoad = '0.0'
         self.diskStats = self._getDiskStats()
         try:
-            with file(_THP_STATE_PATH) as f:
-                s = f.read()
+            with open(_THP_STATE_PATH) as file:
+                s = file.read()
                 self.thpState = s[s.index('[') + 1:s.index(']')]
         except:
             self.thpState = 'never'
diff --git a/vdsm_hooks/nestedvt/before_vm_start.py b/vdsm_hooks/nestedvt/before_vm_start.py
index 03f0f6f..d4fb806 100755
--- a/vdsm_hooks/nestedvt/before_vm_start.py
+++ b/vdsm_hooks/nestedvt/before_vm_start.py
@@ -29,8 +29,8 @@
 for kvm_mod in ("kvm_intel", "kvm_amd"):
     kvm_mod_path = "/sys/module/%s/parameters/nested" % kvm_mod
     try:
-        with file(kvm_mod_path) as f:
-            if f.readline().strip() in ("Y", "1"):
+        with open(kvm_mod_path) as file:
+            if file.readline().strip() in ("Y", "1"):
                 break
     except IOError:
         pass

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8063/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8176/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7273/ : SUCCESS

Antoni Segura Puimedon has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 1: Code-Review+1

Thanks for this. It was long overdue that we'd get rid of the 'file' calling. The only downside is that expanding 'f' to 'file' shadows the file callable (which is considered a bad practice), though since it is shadowing something for which there is little legitimate use, I'm ok with it.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 1: Code-Review-1

Sorry Toni and Dima, shadowing "file" is too brutal for me. Please find a more original variable name. "f" is fine by me.

Dima Kuznetsov has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 2:

Changed file() shadowing, fixed file() in a few more places.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/129... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8363/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7573/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8483/ : SUCCESS

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 2: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/26776/2//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2014-04-15 01:12:58 +0300
Line 4: Commit:     Dima Kuznetsov 
Line 5: CommitDate: 2014-04-29 10:22:37 +0300
Line 6: 
Line 7: open: Change file() to open()
please note that you are removing several of them (but not all).
Line 8: 
Line 9: Update file calls to open calls due to file's deprecation.
Line 10: 
Line 11: Change-Id: I80007044be1c648ebb668704731eae8b83366025

Antoni Segura Puimedon has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3: Code-Review+1

Antoni Segura Puimedon has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3:

The +1 is for those that are here. I don't mind getting the rest in another patch.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/130... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8430/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7640/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8551/ : FAILURE

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3: Code-Review+2

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3: Code-Review+1

please verify.

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3:

Next round please include vdsm_reg dir, like deployUtil and vdsm-reg-setup.

Dima Kuznetsov has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3: Verified+1

Ran nosetest and all the tests ran ok, open to suggestions about additional tests.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 3:

Prudent verification should make sure the relevant code passes are used.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 4:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9406/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/154... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10190/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/910/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10346/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5272/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3430/ : SUCCESS

Antoni Segura Puimedon has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 4: Code-Review+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 4: Code-Review-1

(1 comment)

The patch contain unrelated and wrong change. This is not a replacement of "file" with "open".

http://gerrit.ovirt.org/#/c/26776/4/lib/vdsm/libvirtconnection.py
File lib/vdsm/libvirtconnection.py:

Line 130:         for cred in credentials:
Line 131:             if cred[0] == libvirt.VIR_CRED_AUTHNAME:
Line 132:                 cred[4] = constants.SASL_USERNAME
Line 133:             elif cred[0] == libvirt.VIR_CRED_PASSPHRASE:
Line 134:                 with open(constants.P_VDSM_LIBVIRT_PASSWD) as fileStream:
fileStream? We don't use such term in Python. The typical name for this is "f", which is just fine for a scope of one line. If you want nicer name, call it "file". If you think this is not clear enough, call it "passwd_file".

The previous code open the file and read the first line once. Here you open the file and read it for each item in the credentials list. This is not what the commit message promise.

The correct fix is to replace:

passwd = file(constants.P_VDSM_LIBVIRT_PASSWD).readline().rstrip("\n")

With:

passwd = open(constants.P_VDSM_LIBVIRT_PASSWD).readline().rstrip("\n")

Using with to ensure that the file is always closed is good change but not related to the purpose of this patch. Please make it easy for your reviewers and focus on one logical change.
Line 135:                     cred[4] = fileStream.readline().rstrip("\n")
Line 136:         return 0
Line 137: 
Line 138:     auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE],

Dima Kuznetsov has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/26776/4/lib/vdsm/libvirtconnection.py
File lib/vdsm/libvirtconnection.py:

Line 130:         for cred in credentials:
Line 131:             if cred[0] == libvirt.VIR_CRED_AUTHNAME:
Line 132:                 cred[4] = constants.SASL_USERNAME
Line 133:             elif cred[0] == libvirt.VIR_CRED_PASSPHRASE:
Line 134:                 with open(constants.P_VDSM_LIBVIRT_PASSWD) as fileStream:
> fileStream? We don't use such term in Python. The typical name for this is

thank you, will fix
Line 135:                     cred[4] = fileStream.readline().rstrip("\n")
Line 136:         return 0
Line 137: 
Line 138:     auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE],

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9569/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/155... : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10353/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10510/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5435/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3593/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/971/ : SUCCESS

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5: Code-Review+1

I would split the libvirtconnection and netinfo changes into a separate patch about closing open files explicitly. Replacing file with open there is not very interesting.

Saggi Mizrahi has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5: Code-Review+2

Yeela Kaplan has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5: Code-Review+1

Petr Horáček has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5: Code-Review+1

Yaniv Bronhaim has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5:

verification still missing. will be later on.. won't be in 3.5 for now

Antoni Segura Puimedon has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 5: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 6:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12788/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11842/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1738/ : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/202... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12631/ : SUCCESS

Yeela Kaplan has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 6: Verified+1

Yeela Kaplan has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 6:

verified by: 
ran a vm with a disk on iscsi storage on one host and then migrated the vm to another host and it completed successfully.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 6: Code-Review+2

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: open: Change file() to open()
......................................................................

open: Change file() to open()

Update file calls to open calls due to file's deprecation.

This patch does not convert all calls, more patches will follow.

Change-Id: I80007044be1c648ebb668704731eae8b83366025
Relates-To: https://bugzilla.redhat.com/show_bug.cgi?id=1128715
Signed-off-by: Dima Kuznetsov 
Signed-off-by: Yeela Kaplan 
Reviewed-on: http://gerrit.ovirt.org/26776
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/libvirtconnection.py
M lib/vdsm/netinfo.py
M vdsm/API.py
M vdsm/hooks.py
M vdsm/storage/fileUtils.py
M vdsm/storage/hba.py
M vdsm/virt/sampling.py
M vdsm_hooks/nestedvt/before_vm_start.py
8 files changed, 18 insertions(+), 12 deletions(-)

Approvals:
  Yeela Kaplan: Verified
  Dan Kenigsberg: Looks good to me, approved

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: open: Change file() to open()
......................................................................

Patch Set 7:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4081/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/67/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/95/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/289/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5921/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/92/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/87/ : SUCCESS