Martin Sivák has posted comments on this change.

Change subject: agent: enable global maintenance mode
......................................................................


Patch Set 2:

(3 comments)

Commenting the code a bit more would help and I have inlined couple of comments 
as well.

....................................................
File ovirt_hosted_engine_ha/agent/hosted_engine.py
Line 563:             except ex.MetadataError as e:
Line 564:                 self._log.error(
Line 565:                     str(e),
Line 566:                     extra=self._get_lf_args(self.LF_GLOBAL_MD_ERROR))
Line 567:         self._log.info(
What happens to md variable when parse metadata throws the exception? You log 
the error, but then continue further.
Line 568:             'Global metadata: {0}'.format(md),
Line 569:             extra=self._get_lf_args(self.LF_GLOBAL_MD_UPDATE_DETAIL))
Line 570:         self._global_stats = md
Line 571: 


....................................................
File ovirt_hosted_engine_ha/client/client.py
Line 39:                                 level=logging.CRITICAL)
Line 40:         self._log = logging.getLogger("HAClient")
Line 41:         self._config = None
Line 42: 
Line 43:     def get_all_stats(self, mode='all'):
Can you please make the strings (all, host, global) proper constants and 
document them?
Line 44:         """
Line 45:         Connects to HA broker to get global md and/or host stats, 
based on
Line 46:         mode: 'all', 'global', or 'host'; returns them in a dictionary 
as
Line 47:         {host_id: = {key: value, ...}}


Line 107:         any other flags unaltered.
Line 108:         """
Line 109:         try:
Line 110:             transform_fn = metadata.global_flags[flag]
Line 111:         except:
Please specify the exception types here...
Line 112:             raise Exception('Unknown metadata flag: {0}'.format(flag))
Line 113:         if transform_fn:
Line 114:             put_val = transform_fn(value)
Line 115:         else:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I796d768e197c5fe73646e67297714392bc6e8201
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-hosted-engine-ha
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Martin Sivák <msi...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to