Yaniv Bronhaim has posted comments on this change.

Change subject: API: setHaMaintenanceMode command
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.ovirt.org/#/c/23264/3/client/vdsClient.py
File client/vdsClient.py:

Line 1446: 
Line 1447:     def do_setHaMaintenanceMode(self, args):
Line 1448:         mode = args[0]
Line 1449:         enabled = utils.tobool(args[1])
Line 1450:         stats = self.s.setHaMaintenanceMode(mode, enabled)
consider: 

 assert len(args) == 2
 stats = self.s.setHaMaintenanceMode(args[0], utils.tobool(args[1]))

...
Line 1451:         if stats['status']['code']:
Line 1452:             return stats['status']['code'], 
stats['status']['message']
Line 1453:         return 0, ''
Line 1454: 


Line 2354:                                     'tuning')),
Line 2355:         'setHaMaintenanceMode': (serv.do_setHaMaintenanceMode,
Line 2356:                                  ('<type> <enabled>',
Line 2357:                                   'Set "global" or "local" Hosted 
Engine HA'
Line 2358:                                   ' maintenance')),
global local is the type? and enables needs to be True\False or 0\1 or yes\no ? 
you can write:

<type = global\local> <enabled = True\False> enable or disable HE HA type of 
maintenance. or something like that..
Line 2359:         'deleteImage': (serv.deleteImage,
Line 2360:                         ('<sdUUID> <spUUID> <imgUUID> [<postZero>] 
[<force>]',
Line 2361:                          'Delete Image folder with all volumes.',
Line 2362:                          )),


http://gerrit.ovirt.org/#/c/23264/3/vdsm/API.py
File vdsm/API.py:

Line 1505:             return errCode['haErr']
Line 1506: 
Line 1507:         try:
Line 1508:             haClient.HAClient().set_maintenance_mode(mm, enabled)
Line 1509:         except:
can you be more specific with the catch ? what exceptions can be raised from 
here? try to handle only them and let unexpected exception to raise up
Line 1510:             self.log.exception("error setting HA maintenance mode")
Line 1511:             return errCode['haErr']
Line 1512:         return {'status': doneCode}
Line 1513: 


Line 1507:         try:
Line 1508:             haClient.HAClient().set_maintenance_mode(mm, enabled)
Line 1509:         except:
Line 1510:             self.log.exception("error setting HA maintenance mode")
Line 1511:             return errCode['haErr']
what with the exception? don't you want to know what was it?
Line 1512:         return {'status': doneCode}
Line 1513: 
Line 1514:     # take a rough estimate on how much free mem is available for 
new vm
Line 1515:     # memTotal = memFree + memCached + mem_used_by_non_qemu + 
resident  .


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08c5edb0e9b8cc11eb70ef6a66301335c42aad3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to