Martin Sivák has posted comments on this change. Change subject: packaging: setup: avoid traceback on adding host ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/26887/2/src/plugins/ovirt-hosted-engine-setup/engine/add_host.py File src/plugins/ovirt-hosted-engine-setup/engine/add_host.py: Line 195: self.logger.debug(address) Line 196: return address Line 197: Line 198: def _debug_obj(self, ndict, k, obj): Line 199: if not hasattr(obj, '__dict__'): Why checking and iterating over __dict__? You could have used the vars() builtin: try: for subk,v in vars(obj): _debug_obj(ndict[k], subk, v) except TypeError: ndict[k] = obj But it won't tell you anything about iterables or __slots__ anyway. If you want to see details about list content then you have to use something a bit more complicated. Line 200: ndict[k] = obj Line 201: return Line 202: ndict[k] = {} Line 203: for j in obj.__dict__: Line 498: cluster=cluster_name, Line 499: details=e.detail Line 500: ) Line 501: ) Line 502: if host_added: I would use negative if here and end the method early. You remove one indent level that way. Or even better, I would kill the host_added var and return from the exception handlers. You do not need the variable anyway as you can use else: after the exception block to get the same result. I have real issues with how you split the lines when accessing dicts for example, but since it is already used all over the project I do not object here.. Line 503: up = self._wait_host_ready( Line 504: engine_api, Line 505: self.environment[ohostedcons.EngineEnv.APP_HOST_NAME] Line 506: ) Line 513: ohostedcons.EngineEnv.APP_HOST_NAME Line 514: ], Line 515: ) Line 516: ) Line 517: else: Again, return early, kill the else block and save one indentation level. Line 518: # This works only if the host is up. Line 519: self.logger.debug('Setting CPU for the cluster') Line 520: try: Line 521: cluster = engine_api.clusters.get(cluster_name) -- To view, visit http://gerrit.ovirt.org/26887 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d7bad3033258291c8ab678b41254b1d4798394a Gerrit-PatchSet: 2 Gerrit-Project: ovirt-hosted-engine-setup Gerrit-Branch: master Gerrit-Owner: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Jiří Moskovčák <jmosk...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Lev Veyde <lve...@gmail.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
