Re: [PATCH stable-2.15 15/17] Reduce nesting in LUOobCommand.Exec

2016-12-06 Thread 'Brian Foley' via ganeti-devel
On Tue, Dec 06, 2016 at 10:34:17PM +0100, Iustin Pop wrote:
> On 2016-12-06 17:30:22, Ganeti Development List wrote:
> > This avoids a pylint too-many-nested-blocks warning.
> > 
> > NB it's hard to see from the diff because of all the whitespace, but
> > this just turns the if result.fail_msg check into a guard that continues
> > the next iteration of the loop, and dedends the rest of the loop body
> > rather than having it as an else:
> 
> Actually this is almost readable, because of the empty lines that break
> the patch into multiple chunks.

You're better at this than me then! These kinds of diffs confuse the hell out
of me. :)

Cheers,
Brian.


Re: [PATCH stable-2.15 15/17] Reduce nesting in LUOobCommand.Exec

2016-12-06 Thread Iustin Pop
On 2016-12-06 17:30:22, Ganeti Development List wrote:
> This avoids a pylint too-many-nested-blocks warning.
> 
> NB it's hard to see from the diff because of all the whitespace, but
> this just turns the if result.fail_msg check into a guard that continues
> the next iteration of the loop, and dedends the rest of the loop body
> rather than having it as an else:

Actually this is almost readable, because of the empty lines that break
the patch into multiple chunks.

LGTM.


[PATCH stable-2.15 15/17] Reduce nesting in LUOobCommand.Exec

2016-12-06 Thread 'Brian Foley' via ganeti-devel
This avoids a pylint too-many-nested-blocks warning.

NB it's hard to see from the diff because of all the whitespace, but
this just turns the if result.fail_msg check into a guard that continues
the next iteration of the loop, and dedends the rest of the loop body
rather than having it as an else:

git diff -w will show this.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/misc.py | 65 +++---
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/lib/cmdlib/misc.py b/lib/cmdlib/misc.py
index 62bff52..68ba70f 100644
--- a/lib/cmdlib/misc.py
+++ b/lib/cmdlib/misc.py
@@ -147,43 +147,44 @@ class LUOobCommand(NoHooksLU):
 self.LogWarning("Out-of-band RPC failed on node '%s': %s",
 node.name, result.fail_msg)
 node_entry.append((constants.RS_NODATA, None))
+continue
+
+  try:
+self._CheckPayload(result)
+  except errors.OpExecError, err:
+self.LogWarning("Payload returned by node '%s' is not valid: %s",
+node.name, err)
+node_entry.append((constants.RS_NODATA, None))
   else:
-try:
-  self._CheckPayload(result)
-except errors.OpExecError, err:
-  self.LogWarning("Payload returned by node '%s' is not valid: %s",
-  node.name, err)
-  node_entry.append((constants.RS_NODATA, None))
-else:
-  if self.op.command == constants.OOB_HEALTH:
-# For health we should log important events
-for item, status in result.payload:
-  if status in [constants.OOB_STATUS_WARNING,
-constants.OOB_STATUS_CRITICAL]:
-self.LogWarning("Item '%s' on node '%s' has status '%s'",
-item, node.name, status)
+if self.op.command == constants.OOB_HEALTH:
+  # For health we should log important events
+  for item, status in result.payload:
+if status in [constants.OOB_STATUS_WARNING,
+  constants.OOB_STATUS_CRITICAL]:
+  self.LogWarning("Item '%s' on node '%s' has status '%s'",
+  item, node.name, status)
 
-  if self.op.command == constants.OOB_POWER_ON:
-node.powered = True
-  elif self.op.command == constants.OOB_POWER_OFF:
-node.powered = False
-  elif self.op.command == constants.OOB_POWER_STATUS:
-powered = result.payload[constants.OOB_POWER_STATUS_POWERED]
-if powered != node.powered:
-  logging.warning(("Recorded power state (%s) of node '%s' does 
not"
-   " match actual power state (%s)"), node.powered,
-  node.name, powered)
+if self.op.command == constants.OOB_POWER_ON:
+  node.powered = True
+elif self.op.command == constants.OOB_POWER_OFF:
+  node.powered = False
+elif self.op.command == constants.OOB_POWER_STATUS:
+  powered = result.payload[constants.OOB_POWER_STATUS_POWERED]
+  if powered != node.powered:
+logging.warning(("Recorded power state (%s) of node '%s' does not"
+ " match actual power state (%s)"), node.powered,
+node.name, powered)
 
-  # For configuration changing commands we should update the node
-  if self.op.command in (constants.OOB_POWER_ON,
- constants.OOB_POWER_OFF):
-self.cfg.Update(node, feedback_fn)
+# For configuration changing commands we should update the node
+if self.op.command in (constants.OOB_POWER_ON,
+   constants.OOB_POWER_OFF):
+  self.cfg.Update(node, feedback_fn)
 
-  node_entry.append((constants.RS_NORMAL, result.payload))
+node_entry.append((constants.RS_NORMAL, result.payload))
 
-  if (self.op.command == constants.OOB_POWER_ON and
-  idx < len(self.nodes) - 1):
-time.sleep(self.op.power_delay)
+if (self.op.command == constants.OOB_POWER_ON and
+idx < len(self.nodes) - 1):
+  time.sleep(self.op.power_delay)
 
 return ret
 
-- 
2.8.0.rc3.226.g39d4020