Change in vdsm[master]: gluster: interpret and use correct xml elements

Hello Timothy Asir, Saggi Mizrahi, Aravinda VK, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

http://gerrit.ovirt.org/19674

to review the following change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

gluster: interpret and use correct xml elements

xml output of rebalance and remove-brick status returns filesSkipped
in <skipped> tag and value of <statusStr> tag cleaned up to use it as
proper enum.

Change-Id: I9b90283ac2b9f4b54acfccffa02a30e38e5e9d85
Signed-off-by: Bala.FA 
---
M vdsm/gluster/cli.py
1 file changed, 7 insertions(+), 4 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/19674/1

diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py
index 8349ebe..38269d8 100644
--- a/vdsm/gluster/cli.py
+++ b/vdsm/gluster/cli.py
@@ -19,6 +19,7 @@
 #

import xml.etree.cElementTree as etree
+import re

from vdsm import utils
 from vdsm import netinfo
@@ -611,27 +612,29 @@
     else:
         return

+    st = re.sub('[ -]', '_', tree.find('aggregate/statusStr').text.upper())
     status = {
         'summary': {
             'runtime': float(tree.find('aggregate/runtime').text),
             'filesScanned': int(tree.find('aggregate/lookups').text),
             'filesMoved': int(tree.find('aggregate/files').text),
             'filesFailed': int(tree.find('aggregate/failures').text),
-            'filesSkipped': int(tree.find('aggregate/failures').text),
+            'filesSkipped': int(tree.find('aggregate/skipped').text),
             'totalSizeMoved': int(tree.find('aggregate/size').text),
-            'status': tree.find('aggregate/statusStr').text.upper()},
+            'status': st},
         'hosts': []}

for el in tree.findall('node'):
+        st = re.sub('[ -]', '_', el.find('statusStr').text.upper())
         status['hosts'].append({'name': el.find('nodeName').text,
                                 'id': el.find('id').text,
                                 'runtime': float(el.find('runtime').text),
                                 'filesScanned': int(el.find('lookups').text),
                                 'filesMoved': int(el.find('files').text),
                                 'filesFailed': int(el.find('failures').text),
-                                'filesSkipped': int(el.find('failures').text),
+                                'filesSkipped': int(el.find('skipped').text),
                                 'totalSizeMoved': int(el.find('size').text),
-                                'status': el.find('statusStr').text.upper()})
+                                'status': st})

return status

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3806/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4691/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4615/ : SUCCESS

Aravinda VK has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 1: Code-Review-1

(1 comment)

....................................................
File vdsm/gluster/cli.py
Line 611:         tree = xmltree.find('volRemoveBrick')
Line 612:     else:
Line 613:         return
Line 614: 
Line 615:     st = re.sub('[ -]', '_', tree.find('aggregate/statusStr').text.upper())
We can use simple string replace instead of re. If we are using re, "\s" is better representation for space for readability.

re.sub('\s|-', '_', tree.find('aggregate/statusStr').text.upper())
Line 616:     status = {
Line 617:         'summary': {
Line 618:             'runtime': float(tree.find('aggregate/runtime').text),
Line 619:             'filesScanned': int(tree.find('aggregate/lookups').text),

Saggi Mizrahi has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 1:

(1 comment)

....................................................
File vdsm/gluster/cli.py
Line 611:         tree = xmltree.find('volRemoveBrick')
Line 612:     else:
Line 613:         return
Line 614: 
Line 615:     st = re.sub('[ -]', '_', tree.find('aggregate/statusStr').text.upper())
what's wrong with str.replace()?
Line 616:     status = {
Line 617:         'summary': {
Line 618:             'runtime': float(tree.find('aggregate/runtime').text),
Line 619:             'filesScanned': int(tree.find('aggregate/lookups').text),

Bala.FA has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 1:

(1 comment)

....................................................
File vdsm/gluster/cli.py
Line 611:         tree = xmltree.find('volRemoveBrick')
Line 612:     else:
Line 613:         return
Line 614: 
Line 615:     st = re.sub('[ -]', '_', tree.find('aggregate/statusStr').text.upper())
I need two chained replace().  To avoid that I used regex.  Let me know I go back to chained replace().
Line 616:     status = {
Line 617:         'summary': {
Line 618:             'runtime': float(tree.find('aggregate/runtime').text),
Line 619:             'filesScanned': int(tree.find('aggregate/lookups').text),

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 2: Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5064/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4990/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4183/ : FAILURE

Bala.FA has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 3: Verified+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 3: Code-Review-1 Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5010/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4206/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5084/ : FAILURE

Bala.FA has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 4: Verified+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 4: Code-Review-1

Build Unstable

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4644/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5444/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5523/ : SUCCESS

Bala.FA has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 6: Verified+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 5: Code-Review-1

Build Unstable

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4647/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5447/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5526/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 6: Code-Review-1

Build Unstable

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4649/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5449/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5528/ : SUCCESS

Timothy Asir has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 6:

(1 comment)

....................................................
File vdsm/gluster/cli.py
Line 645:                                 'filesFailed': el.find('failures').text,
Line 646:                                 'filesSkipped': el.find('skipped').text,
Line 647:                                 'totalSizeMoved': el.find('size').text,
Line 648:                                 'status': statusStr.upper()})
Line 649: 
Remove this extra blank line
Line 650: 
Line 651:     return status
Line 652: 
Line 653:

Bala.FA has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 8: Verified+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 7:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4656/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5456/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5535/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 8:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4657/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5457/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5536/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 9:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4708/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5508/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5587/ : SUCCESS

Timothy Asir has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 9: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 10:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6591/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5698/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6504/ : SUCCESS

Bala.FA has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 11: Verified+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: gluster: interpret and use correct xml elements
......................................................................

Patch Set 11: Code-Review+2

Copying and escalating Timothy's score of ps9.

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: gluster: interpret and use correct xml elements
......................................................................

gluster: interpret and use correct xml elements

xml output of rebalance and remove-brick status returns filesSkipped
in <skipped> tag and value of <statusStr> tag cleaned up to use it as
proper enum.

Change-Id: I9b90283ac2b9f4b54acfccffa02a30e38e5e9d85
Signed-off-by: Bala.FA 
Reviewed-on: http://gerrit.ovirt.org/19674
Reviewed-by: Dan Kenigsberg 
---
M vdsm/gluster/cli.py
1 file changed, 8 insertions(+), 4 deletions(-)

Approvals:
  Bala.FA: Verified
  Dan Kenigsberg: Looks good to me, approved