)]}'
{".gitignore":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":1,"context_line":".pydevproject"},{"line_number":2,"context_line":".project"},{"line_number":3,"context_line":"*.pyc"},{"line_number":4,"context_line":"*.DS_Store"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAAEn%2F%2F%2Bwk%3D","line":1,"updated":"2012-02-04 01:49:11.000000000","message":"git supports global ignore so that each developer\u0027s personal ignores don\u0027t have to creep into the project\u0027s ignore list:\n\nhttp://jqr.github.com/2009/02/03/global-git-ignore.html","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":1,"context_line":".pydevproject"},{"line_number":2,"context_line":".project"},{"line_number":3,"context_line":"*.pyc"},{"line_number":4,"context_line":"*.DS_Store"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAAEn%2F%2F%2BGo%3D","line":1,"in_reply_to":"AAAAEn%2F%2F%2Bwk%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"}],"nova/api/openstack/compute/contrib/aggregates.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        try:"},{"line_number":86,"context_line":"            aggregate \u003d self.api.get_aggregate(context, id)"},{"line_number":87,"context_line":"        except exception.AggregateNotFound:"},{"line_number":88,"context_line":"            LOG.exception(_(\"Cannot show aggregate: %(id)s\" % locals()))"},{"line_number":89,"context_line":"            raise exc.HTTPNotFound"},{"line_number":90,"context_line":"        return self._marshall_aggregate(aggregate)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B14%3D","line":88,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        try:"},{"line_number":86,"context_line":"            aggregate \u003d self.api.get_aggregate(context, id)"},{"line_number":87,"context_line":"        except exception.AggregateNotFound:"},{"line_number":88,"context_line":"            LOG.exception(_(\"Cannot show aggregate: %(id)s\" % locals()))"},{"line_number":89,"context_line":"            raise exc.HTTPNotFound"},{"line_number":90,"context_line":"        return self._marshall_aggregate(aggregate)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGk%3D","line":88,"in_reply_to":"AAAAEn%2F%2F%2B14%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            aggregate \u003d self.api.update_aggregate(context, id, updates)"},{"line_number":113,"context_line":"        except exception.AggregateNotFound:"},{"line_number":114,"context_line":"            LOG.exception(_(\"Cannot update aggregate: %(id)s\" % locals()))"},{"line_number":115,"context_line":"            raise exc.HTTPNotFound"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        return self._marshall_aggregate(aggregate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B10%3D","line":114,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            aggregate \u003d self.api.update_aggregate(context, id, updates)"},{"line_number":113,"context_line":"        except exception.AggregateNotFound:"},{"line_number":114,"context_line":"            LOG.exception(_(\"Cannot update aggregate: %(id)s\" % locals()))"},{"line_number":115,"context_line":"            raise exc.HTTPNotFound"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        return self._marshall_aggregate(aggregate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGg%3D","line":114,"in_reply_to":"AAAAEn%2F%2F%2B10%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            self.api.delete_aggregate(context, id)"},{"line_number":125,"context_line":"        except exception.AggregateNotFound:"},{"line_number":126,"context_line":"            LOG.exception(_(\"Cannot delete aggregate: %(id)s\" % locals()))"},{"line_number":127,"context_line":"            raise exc.HTTPNotFound"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def action(self, req, id, body):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B1s%3D","line":126,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            self.api.delete_aggregate(context, id)"},{"line_number":125,"context_line":"        except exception.AggregateNotFound:"},{"line_number":126,"context_line":"            LOG.exception(_(\"Cannot delete aggregate: %(id)s\" % locals()))"},{"line_number":127,"context_line":"            raise exc.HTTPNotFound"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def action(self, req, id, body):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGc%3D","line":126,"in_reply_to":"AAAAEn%2F%2F%2B1s%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        try:"},{"line_number":150,"context_line":"            aggregate \u003d self.api.add_host_to_aggregate(context, id, host)"},{"line_number":151,"context_line":"        except (exception.AggregateNotFound, exception.ComputeHostNotFound):"},{"line_number":152,"context_line":"            LOG.exception(_(\"Cannot add host %(host)s in aggregate \""},{"line_number":153,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":154,"context_line":"            raise exc.HTTPNotFound"},{"line_number":155,"context_line":"        except (exception.AggregateHostConflict,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B1k%3D","line":152,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        try:"},{"line_number":150,"context_line":"            aggregate \u003d self.api.add_host_to_aggregate(context, id, host)"},{"line_number":151,"context_line":"        except (exception.AggregateNotFound, exception.ComputeHostNotFound):"},{"line_number":152,"context_line":"            LOG.exception(_(\"Cannot add host %(host)s in aggregate \""},{"line_number":153,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":154,"context_line":"            raise exc.HTTPNotFound"},{"line_number":155,"context_line":"        except (exception.AggregateHostConflict,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGY%3D","line":152,"in_reply_to":"AAAAEn%2F%2F%2B1k%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        except (exception.AggregateHostConflict,"},{"line_number":156,"context_line":"                exception.AggregateHostExists,"},{"line_number":157,"context_line":"                exception.InvalidAggregateAction):"},{"line_number":158,"context_line":"            LOG.exception(_(\"Cannot add host %(host)s in aggregate \""},{"line_number":159,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":160,"context_line":"            raise exc.HTTPConflict"},{"line_number":161,"context_line":"        return self._marshall_aggregate(aggregate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B1g%3D","line":158,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        except (exception.AggregateHostConflict,"},{"line_number":156,"context_line":"                exception.AggregateHostExists,"},{"line_number":157,"context_line":"                exception.InvalidAggregateAction):"},{"line_number":158,"context_line":"            LOG.exception(_(\"Cannot add host %(host)s in aggregate \""},{"line_number":159,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":160,"context_line":"            raise exc.HTTPConflict"},{"line_number":161,"context_line":"        return self._marshall_aggregate(aggregate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGU%3D","line":158,"in_reply_to":"AAAAEn%2F%2F%2B1g%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        try:"},{"line_number":169,"context_line":"            aggregate \u003d self.api.remove_host_from_aggregate(context, id, host)"},{"line_number":170,"context_line":"        except (exception.AggregateNotFound, exception.AggregateHostNotFound):"},{"line_number":171,"context_line":"            LOG.exception(_(\"Cannot remove host %(host)s in aggregate \""},{"line_number":172,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":173,"context_line":"            raise exc.HTTPNotFound"},{"line_number":174,"context_line":"        except exception.InvalidAggregateAction:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B1c%3D","line":171,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        try:"},{"line_number":169,"context_line":"            aggregate \u003d self.api.remove_host_from_aggregate(context, id, host)"},{"line_number":170,"context_line":"        except (exception.AggregateNotFound, exception.AggregateHostNotFound):"},{"line_number":171,"context_line":"            LOG.exception(_(\"Cannot remove host %(host)s in aggregate \""},{"line_number":172,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":173,"context_line":"            raise exc.HTTPNotFound"},{"line_number":174,"context_line":"        except exception.InvalidAggregateAction:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGQ%3D","line":171,"in_reply_to":"AAAAEn%2F%2F%2B1c%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":173,"context_line":"            raise exc.HTTPNotFound"},{"line_number":174,"context_line":"        except exception.InvalidAggregateAction:"},{"line_number":175,"context_line":"            LOG.exception(_(\"Cannot remove host %(host)s in aggregate \""},{"line_number":176,"context_line":"                \"%(id)s\" % locals()))"},{"line_number":177,"context_line":"            raise exc.HTTPConflict"},{"line_number":178,"context_line":"        return self._marshall_aggregate(aggregate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B1Y%3D","line":175,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":173,"context_line":"            raise exc.HTTPNotFound"},{"line_number":174,"context_line":"        except exception.InvalidAggregateAction:"},{"line_number":175,"context_line":"            LOG.exception(_(\"Cannot remove host %(host)s in aggregate \""},{"line_number":176,"context_line":"                \"%(id)s\" % locals()))"},{"line_number":177,"context_line":"            raise exc.HTTPConflict"},{"line_number":178,"context_line":"        return self._marshall_aggregate(aggregate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGM%3D","line":175,"in_reply_to":"AAAAEn%2F%2F%2B1Y%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":192,"context_line":"            aggregate \u003d self.api.update_aggregate_metadata(context,"},{"line_number":193,"context_line":"                                                           id, metadata)"},{"line_number":194,"context_line":"        except exception.AggregateNotFound:"},{"line_number":195,"context_line":"            LOG.exception(_(\"Cannot set metadata %(metadata)s in aggregate \""},{"line_number":196,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":197,"context_line":"            raise exc.HTTPNotFound"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B1U%3D","line":195,"updated":"2012-02-04 01:49:11.000000000","message":"String formatting should be performed outside of the localization call.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":192,"context_line":"            aggregate \u003d self.api.update_aggregate_metadata(context,"},{"line_number":193,"context_line":"                                                           id, metadata)"},{"line_number":194,"context_line":"        except exception.AggregateNotFound:"},{"line_number":195,"context_line":"            LOG.exception(_(\"Cannot set metadata %(metadata)s in aggregate \""},{"line_number":196,"context_line":"                            \"%(id)s\" % locals()))"},{"line_number":197,"context_line":"            raise exc.HTTPNotFound"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BGI%3D","line":195,"in_reply_to":"AAAAEn%2F%2F%2B1U%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"}],"nova/compute/manager.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":2276,"context_line":"            self.driver.add_to_aggregate(context, aggregate, host, **kwargs)"},{"line_number":2277,"context_line":"        except exception.AggregateError:"},{"line_number":2278,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2279,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2280,"context_line":"            # UNDO"},{"line_number":2281,"context_line":"            self.db.aggregate_host_delete(context, aggregate_id, host)"},{"line_number":2282,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BxE%3D","line":2279,"updated":"2012-02-04 01:49:11.000000000","message":"If the first db update fails, should the second one not be attempted?  If either of the db updates fail, is it acceptable that a db-related exception prevent the AggregateError from being re-raised?","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":2276,"context_line":"            self.driver.add_to_aggregate(context, aggregate, host, **kwargs)"},{"line_number":2277,"context_line":"        except exception.AggregateError:"},{"line_number":2278,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2279,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2280,"context_line":"            # UNDO"},{"line_number":2281,"context_line":"            self.db.aggregate_host_delete(context, aggregate_id, host)"},{"line_number":2282,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BFQ%3D","line":2279,"in_reply_to":"AAAAEn%2F%2F%2BxE%3D","updated":"2012-02-07 17:24:52.000000000","message":"the AggregateError exception is raised in case the driver operation fails (e.g. hypervisor pool setup failure), so it\u0027s assumed that there is nothing wrong with interacting with the DB. \n\nThe reason of undoing in case of error is because the host model object is added in the nova-api space, whereas here we are in the nova-compute space. It could be argued that we should make changes to the model only if the whole operation succeed, but it seemed reasonable to be \u0027optimistic\u0027 and share the load between the parties involved in the aggregate setup.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":2277,"context_line":"        except exception.AggregateError:"},{"line_number":2278,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2279,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2280,"context_line":"            # UNDO"},{"line_number":2281,"context_line":"            self.db.aggregate_host_delete(context, aggregate_id, host)"},{"line_number":2282,"context_line":"            raise"},{"line_number":2283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BxM%3D","line":2280,"updated":"2012-02-04 01:49:11.000000000","message":"Unnecessary comment.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":2277,"context_line":"        except exception.AggregateError:"},{"line_number":2278,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2279,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2280,"context_line":"            # UNDO"},{"line_number":2281,"context_line":"            self.db.aggregate_host_delete(context, aggregate_id, host)"},{"line_number":2282,"context_line":"            raise"},{"line_number":2283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BFk%3D","line":2280,"in_reply_to":"AAAAEn%2F%2F%2BxM%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":2289,"context_line":"            self.driver.remove_from_aggregate(context,"},{"line_number":2290,"context_line":"                                              aggregate, host, **kwargs)"},{"line_number":2291,"context_line":"        except exception.AggregateError:"},{"line_number":2292,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2293,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2294,"context_line":"            # UNDO"},{"line_number":2295,"context_line":"            self.db.aggregate_host_add(context, aggregate_id, host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bw4%3D","line":2292,"updated":"2012-02-04 01:49:11.000000000","message":"The comments from add_aggregate_host apply equally to this block.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":2289,"context_line":"            self.driver.remove_from_aggregate(context,"},{"line_number":2290,"context_line":"                                              aggregate, host, **kwargs)"},{"line_number":2291,"context_line":"        except exception.AggregateError:"},{"line_number":2292,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2293,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2294,"context_line":"            # UNDO"},{"line_number":2295,"context_line":"            self.db.aggregate_host_add(context, aggregate_id, host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BFg%3D","line":2292,"in_reply_to":"AAAAEn%2F%2F%2Bw4%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":2285,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2286,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2287,"context_line":"            self.db.aggregate_host_delete(context, aggregate_id, host)"},{"line_number":2288,"context_line":"            raise"},{"line_number":2289,"context_line":""},{"line_number":2290,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":2291,"context_line":"    def remove_aggregate_host(self, context, aggregate_id, host, **kwargs):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F99I%3D","line":2288,"updated":"2012-02-07 20:11:21.000000000","message":"If it is important that the AggregateError be raised even if the db updates result raise an exception, consider saving the exception state before the updates (via sys.exc_info) and trapping db-related exceptions to allow the saved exception state to always be re-raised.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":2285,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2286,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2287,"context_line":"            self.db.aggregate_host_delete(context, aggregate_id, host)"},{"line_number":2288,"context_line":"            raise"},{"line_number":2289,"context_line":""},{"line_number":2290,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":2291,"context_line":"    def remove_aggregate_host(self, context, aggregate_id, host, **kwargs):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F9WA%3D","line":2288,"in_reply_to":"AAAAEn%2F%2F99I%3D","updated":"2012-02-08 17:14:44.000000000","message":"good point, thanks!","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2299,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2300,"context_line":"            self.db.aggregate_host_add(context, aggregate_id, host)"},{"line_number":2301,"context_line":"            raise"},{"line_number":2302,"context_line":""},{"line_number":2303,"context_line":"    @manager.periodic_task("},{"line_number":2304,"context_line":"        ticks_between_runs\u003dFLAGS.image_cache_manager_interval)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F99E%3D","line":2301,"updated":"2012-02-07 20:11:21.000000000","message":"Same comment as add_aggregate_host.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"            status \u003d {\u0027operational_state\u0027: aggregate_states.ERROR}"},{"line_number":2299,"context_line":"            self.db.aggregate_update(context, aggregate.id, status)"},{"line_number":2300,"context_line":"            self.db.aggregate_host_add(context, aggregate_id, host)"},{"line_number":2301,"context_line":"            raise"},{"line_number":2302,"context_line":""},{"line_number":2303,"context_line":"    @manager.periodic_task("},{"line_number":2304,"context_line":"        ticks_between_runs\u003dFLAGS.image_cache_manager_interval)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F9V8%3D","line":2301,"in_reply_to":"AAAAEn%2F%2F99E%3D","updated":"2012-02-08 17:14:44.000000000","message":"good point, thanks!","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":2267,"context_line":"                self._set_instance_error_state(context, instance_uuid)"},{"line_number":2268,"context_line":""},{"line_number":2269,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":2270,"context_line":"    def add_aggregate_host(self, context, aggregate_id, host, **kwargs):"},{"line_number":2271,"context_line":"        \"\"\"Adds a host to a physical hypervisor pool.\"\"\""},{"line_number":2272,"context_line":"        aggregate \u003d self.db.aggregate_get(context, aggregate_id)"},{"line_number":2273,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F89U%3D","line":2270,"updated":"2012-02-09 00:32:28.000000000","message":"Will driver.add_to_aggregate raise AggregateError if a host already belongs to an aggregate and add_to_aggregate is called again for that host?  If so, consider adding special handling for that case to avoid an unnecessary db update.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":2267,"context_line":"                self._set_instance_error_state(context, instance_uuid)"},{"line_number":2268,"context_line":""},{"line_number":2269,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":2270,"context_line":"    def add_aggregate_host(self, context, aggregate_id, host, **kwargs):"},{"line_number":2271,"context_line":"        \"\"\"Adds a host to a physical hypervisor pool.\"\"\""},{"line_number":2272,"context_line":"        aggregate \u003d self.db.aggregate_get(context, aggregate_id)"},{"line_number":2273,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F8uM%3D","line":2270,"in_reply_to":"AAAAEn%2F%2F89U%3D","updated":"2012-02-09 16:57:04.000000000","message":"We don\u0027t get to compute; this is checked at the api layer.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"31108f6e60b0809f3767fd2031947ca64f08814c","unresolved":false,"context_lines":[{"line_number":2267,"context_line":"                self._set_instance_error_state(context, instance_uuid)"},{"line_number":2268,"context_line":""},{"line_number":2269,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":2270,"context_line":"    def add_aggregate_host(self, context, aggregate_id, host, **kwargs):"},{"line_number":2271,"context_line":"        \"\"\"Adds a host to a physical hypervisor pool.\"\"\""},{"line_number":2272,"context_line":"        aggregate \u003d self.db.aggregate_get(context, aggregate_id)"},{"line_number":2273,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F8oQ%3D","line":2270,"in_reply_to":"AAAAEn%2F%2F8uM%3D","updated":"2012-02-09 19:32:30.000000000","message":"I\u0027ll assume this means a duplicate add will not raise an exception.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"            self.db.aggregate_update(context, aggregate_id, status)"},{"line_number":2299,"context_line":"            op(context, aggregate_id, host)"},{"line_number":2300,"context_line":"        except Exception:"},{"line_number":2301,"context_line":"            LOG.exception(_(\u0027Aggregate %(aggregate_id)s: unrecoverable state \u0027"},{"line_number":2302,"context_line":"                            \u0027during operation on %(host)s\u0027) % locals())"},{"line_number":2303,"context_line":""},{"line_number":2304,"context_line":"    @manager.periodic_task("}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F89c%3D","line":2301,"updated":"2012-02-09 00:32:28.000000000","message":"Consider including more detail about the exception to aid in debugging.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"            self.db.aggregate_update(context, aggregate_id, status)"},{"line_number":2299,"context_line":"            op(context, aggregate_id, host)"},{"line_number":2300,"context_line":"        except Exception:"},{"line_number":2301,"context_line":"            LOG.exception(_(\u0027Aggregate %(aggregate_id)s: unrecoverable state \u0027"},{"line_number":2302,"context_line":"                            \u0027during operation on %(host)s\u0027) % locals())"},{"line_number":2303,"context_line":""},{"line_number":2304,"context_line":"    @manager.periodic_task("}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F8uc%3D","line":2301,"in_reply_to":"AAAAEn%2F%2F89c%3D","updated":"2012-02-09 16:57:04.000000000","message":"the exception gets logged, doesn\u0027t it? So the stacktrace should be enough. Or were you referring to something else?","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"31108f6e60b0809f3767fd2031947ca64f08814c","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"            self.db.aggregate_update(context, aggregate_id, status)"},{"line_number":2299,"context_line":"            op(context, aggregate_id, host)"},{"line_number":2300,"context_line":"        except Exception:"},{"line_number":2301,"context_line":"            LOG.exception(_(\u0027Aggregate %(aggregate_id)s: unrecoverable state \u0027"},{"line_number":2302,"context_line":"                            \u0027during operation on %(host)s\u0027) % locals())"},{"line_number":2303,"context_line":""},{"line_number":2304,"context_line":"    @manager.periodic_task("}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F8oU%3D","line":2301,"in_reply_to":"AAAAEn%2F%2F8uc%3D","updated":"2012-02-09 19:32:30.000000000","message":"Ah, that\u0027s my ignorance.  I didn\u0027t realize that LOG.exception automatically called sys.exc_info().","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"54e56d82d9c112f09a2ead446c191f622138da8b","unresolved":false,"context_lines":[{"line_number":2306,"context_line":"            error \u003d sys.exc_info()"},{"line_number":2307,"context_line":"            self._undo_aggregate_operation(context, self.db.aggregate_host_add,"},{"line_number":2308,"context_line":"                                           aggregate.id, host)"},{"line_number":2309,"context_line":"            raise error[0], error[1], error[2]"},{"line_number":2310,"context_line":""},{"line_number":2311,"context_line":"    def _undo_aggregate_operation(self, context, op, aggregate_id, host):"},{"line_number":2312,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F4Nw%3D","line":2309,"updated":"2012-02-21 22:16:50.000000000","message":"Please correct me if I\u0027m wrong here, but can\u0027t you just use a bare \u0027raise\u0027 here? Why do you need to call sys.exc_info() and re-raise each individual part?","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"39a609730d2e40e6f4f863402ba6879a25ac0a52","unresolved":false,"context_lines":[{"line_number":2306,"context_line":"            error \u003d sys.exc_info()"},{"line_number":2307,"context_line":"            self._undo_aggregate_operation(context, self.db.aggregate_host_add,"},{"line_number":2308,"context_line":"                                           aggregate.id, host)"},{"line_number":2309,"context_line":"            raise error[0], error[1], error[2]"},{"line_number":2310,"context_line":""},{"line_number":2311,"context_line":"    def _undo_aggregate_operation(self, context, op, aggregate_id, host):"},{"line_number":2312,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F4NE%3D","line":2309,"in_reply_to":"AAAAEn%2F%2F4Nw%3D","updated":"2012-02-21 22:29:39.000000000","message":"if _undo_aggregate_operation causes an exception wouldn\u0027t a bare \u0027raise\u0027 raise the newly occurred exception? That is my understanding. I could use\n\nutils.save_and_reraise_exception()\n\nwhich I believe it does the same thing.","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"63cea0763ac55a738164688f9bc8326c68d14929","unresolved":false,"context_lines":[{"line_number":3354,"context_line":"    {\u0027extra_specs\u0027 : {\u0027k1\u0027: \u0027v1\u0027, \u0027k2\u0027: \u0027v2\u0027, ...}}"},{"line_number":3355,"context_line":""},{"line_number":3356,"context_line":"    \"\"\""},{"line_number":3357,"context_line":"    session \u003d get_session()"},{"line_number":3358,"context_line":"    with session.begin():"},{"line_number":3359,"context_line":"        try:"},{"line_number":3360,"context_line":"            instance_type_get_by_name(context, values[\u0027name\u0027], session)"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6so%3D","line":3357,"updated":"2012-02-14 22:36:48.000000000","message":"This code appears to be trying to enforce uniqueness constraints on the name and flavorid fields.  Unfortunately, the chosen approach has a clear race condition.  If two competing processes pass the checks before either commits, both additions will subsequently succeed.  The only way to ensure the uniqueness of these two columns is to declare the columns unique in the model and create a corresponding database migration.\n\nThis comment applies equally to the approach taken in volume_type_create().","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3eae67bad935d44c0b50f30f3c660660f96c56a0","unresolved":false,"context_lines":[{"line_number":3354,"context_line":"    {\u0027extra_specs\u0027 : {\u0027k1\u0027: \u0027v1\u0027, \u0027k2\u0027: \u0027v2\u0027, ...}}"},{"line_number":3355,"context_line":""},{"line_number":3356,"context_line":"    \"\"\""},{"line_number":3357,"context_line":"    session \u003d get_session()"},{"line_number":3358,"context_line":"    with session.begin():"},{"line_number":3359,"context_line":"        try:"},{"line_number":3360,"context_line":"            instance_type_get_by_name(context, values[\u0027name\u0027], session)"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6rY%3D","line":3357,"in_reply_to":"AAAAEn%2F%2F6so%3D","updated":"2012-02-14 22:51:17.000000000","message":"I don\u0027t follow, that\u0027s not my code.","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"63cea0763ac55a738164688f9bc8326c68d14929","unresolved":false,"context_lines":[{"line_number":3478,"context_line":"        instance_type_ref \u003d instance_type_get_by_name(context, name,"},{"line_number":3479,"context_line":"                                                      session\u003dsession)"},{"line_number":3480,"context_line":"        instance_type_id \u003d instance_type_ref[\u0027id\u0027]"},{"line_number":3481,"context_line":"        session.query(models.InstanceTypes).\\"},{"line_number":3482,"context_line":"                filter_by(id\u003dinstance_type_id).\\"},{"line_number":3483,"context_line":"                update({\u0027deleted\u0027: True,"},{"line_number":3484,"context_line":"                        \u0027deleted_at\u0027: utils.utcnow(),"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6sk%3D","line":3481,"updated":"2012-02-14 22:36:48.000000000","message":"Please avoid using backslashes for line continuation.","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3eae67bad935d44c0b50f30f3c660660f96c56a0","unresolved":false,"context_lines":[{"line_number":3478,"context_line":"        instance_type_ref \u003d instance_type_get_by_name(context, name,"},{"line_number":3479,"context_line":"                                                      session\u003dsession)"},{"line_number":3480,"context_line":"        instance_type_id \u003d instance_type_ref[\u0027id\u0027]"},{"line_number":3481,"context_line":"        session.query(models.InstanceTypes).\\"},{"line_number":3482,"context_line":"                filter_by(id\u003dinstance_type_id).\\"},{"line_number":3483,"context_line":"                update({\u0027deleted\u0027: True,"},{"line_number":3484,"context_line":"                        \u0027deleted_at\u0027: utils.utcnow(),"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6rQ%3D","line":3481,"in_reply_to":"AAAAEn%2F%2F6sk%3D","updated":"2012-02-14 22:51:17.000000000","message":"ditto","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"54e56d82d9c112f09a2ead446c191f622138da8b","unresolved":false,"context_lines":[{"line_number":4272,"context_line":"    try:"},{"line_number":4273,"context_line":"        aggregate \u003d models.Aggregate()"},{"line_number":4274,"context_line":"        aggregate.update(values)"},{"line_number":4275,"context_line":"        if \u0027operational_state\u0027 not in values:"},{"line_number":4276,"context_line":"            aggregate.operational_state \u003d aggregate_states.CREATED"},{"line_number":4277,"context_line":"        aggregate.save()"},{"line_number":4278,"context_line":"    except exception.DBError:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F4Ns%3D","line":4275,"updated":"2012-02-21 22:16:50.000000000","message":"This would be cleaner to call \u0027values.setdefault(\u0027operational_state\u0027, aggregate_states.CREATED) before the update rather than using a conditional after the call to update().","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"39a609730d2e40e6f4f863402ba6879a25ac0a52","unresolved":false,"context_lines":[{"line_number":4272,"context_line":"    try:"},{"line_number":4273,"context_line":"        aggregate \u003d models.Aggregate()"},{"line_number":4274,"context_line":"        aggregate.update(values)"},{"line_number":4275,"context_line":"        if \u0027operational_state\u0027 not in values:"},{"line_number":4276,"context_line":"            aggregate.operational_state \u003d aggregate_states.CREATED"},{"line_number":4277,"context_line":"        aggregate.save()"},{"line_number":4278,"context_line":"    except exception.DBError:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F4NA%3D","line":4275,"in_reply_to":"AAAAEn%2F%2F4Ns%3D","updated":"2012-02-21 22:29:39.000000000","message":"good point. Done","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"}],"nova/db/sqlalchemy/models.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":903,"context_line":"                          primaryjoin\u003d\u0027and_(\u0027"},{"line_number":904,"context_line":"                                 \u0027Aggregate.id \u003d\u003d AggregateHost.aggregate_id,\u0027"},{"line_number":905,"context_line":"                                 \u0027AggregateHost.deleted \u003d\u003d False,\u0027"},{"line_number":906,"context_line":"                                 \u0027Aggregate.deleted \u003d\u003d False)\u0027,"},{"line_number":907,"context_line":"                         secondaryjoin\u003d\u0027and_(\u0027"},{"line_number":908,"context_line":"                                \u0027AggregateHost.aggregate_id \u003d\u003d Aggregate.id, \u0027"},{"line_number":909,"context_line":"                                \u0027Aggregate.deleted \u003d\u003d False)\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B0c%3D","line":906,"updated":"2012-02-04 01:49:11.000000000","message":"Is the inclusion of this clause intentional?  For consistency, I would expect the backref\u0027s join to similarly require AggregateHost.deleted to be False.  Alternately, neither join should be testing the primary model\u0027s deleted status.  \n\nThis comment applies equally to the definition of the _metadata relationship.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":903,"context_line":"                          primaryjoin\u003d\u0027and_(\u0027"},{"line_number":904,"context_line":"                                 \u0027Aggregate.id \u003d\u003d AggregateHost.aggregate_id,\u0027"},{"line_number":905,"context_line":"                                 \u0027AggregateHost.deleted \u003d\u003d False,\u0027"},{"line_number":906,"context_line":"                                 \u0027Aggregate.deleted \u003d\u003d False)\u0027,"},{"line_number":907,"context_line":"                         secondaryjoin\u003d\u0027and_(\u0027"},{"line_number":908,"context_line":"                                \u0027AggregateHost.aggregate_id \u003d\u003d Aggregate.id, \u0027"},{"line_number":909,"context_line":"                                \u0027Aggregate.deleted \u003d\u003d False)\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BFE%3D","line":906,"in_reply_to":"AAAAEn%2F%2F%2B0c%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":921,"context_line":"                         backref\u003d\u0027aggregates\u0027)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"    @property"},{"line_number":924,"context_line":"    def hosts(self):"},{"line_number":925,"context_line":"        return [h.host for h in self._hosts]"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B00%3D","line":924,"updated":"2012-02-04 01:49:11.000000000","message":"Are the _hosts and _metadata relationships intended to be used for anything other than the host and metadetails properties?  If not, consider implementing the properties using direct sqlalchemy queries so as to avoid the overhead of orm object creation.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":921,"context_line":"                         backref\u003d\u0027aggregates\u0027)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"    @property"},{"line_number":924,"context_line":"    def hosts(self):"},{"line_number":925,"context_line":"        return [h.host for h in self._hosts]"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BEs%3D","line":924,"in_reply_to":"AAAAEn%2F%2F%2B00%3D","updated":"2012-02-07 17:24:52.000000000","message":"the current use case is to expose object properties, and you\u0027re right it may be improved, but it feels to early to carry out this optimization. I hope that\u0027s okay.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"    connection is lost and needs to be reestablished."},{"line_number":1006,"context_line":"    \"\"\""},{"line_number":1007,"context_line":"    from sqlalchemy import create_engine"},{"line_number":1008,"context_line":"    models \u003d (AgentBuild, Aggregate, AggregateHost, AggregateMetadata,"},{"line_number":1009,"context_line":"              AuthToken,"},{"line_number":1010,"context_line":"              Certificate, Console, ConsolePool,"},{"line_number":1011,"context_line":"              FixedIp, FloatingIp,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2B0E%3D","line":1008,"updated":"2012-02-04 01:49:11.000000000","message":"Why are the models being reorganized in this way?  Consider using one line per model to ensure that model additions and deletions are easier to discern when using simple diff tools.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"    connection is lost and needs to be reestablished."},{"line_number":1006,"context_line":"    \"\"\""},{"line_number":1007,"context_line":"    from sqlalchemy import create_engine"},{"line_number":1008,"context_line":"    models \u003d (AgentBuild, Aggregate, AggregateHost, AggregateMetadata,"},{"line_number":1009,"context_line":"              AuthToken,"},{"line_number":1010,"context_line":"              Certificate, Console, ConsolePool,"},{"line_number":1011,"context_line":"              FixedIp, FloatingIp,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BEc%3D","line":1008,"in_reply_to":"AAAAEn%2F%2F%2B0E%3D","updated":"2012-02-07 17:24:52.000000000","message":"Fair enough, I just wanted to order them alphabetically because it\u0027s difficult to see which are are in and which aren\u0027t. I\u0027ll use one line per model.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"61813786fae91d2235d0aa07cac09c3197682837","unresolved":false,"context_lines":[{"line_number":932,"context_line":""},{"line_number":933,"context_line":"    @property"},{"line_number":934,"context_line":"    def metadetails(self):"},{"line_number":935,"context_line":"        return dict([(m.key, m.value) for m in self._metadata])"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"class AgentBuild(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAEn%2F%2F5Us%3D","line":935,"updated":"2012-02-17 19:49:15.000000000","message":"I don\u0027t see any corresponding database migration.  Did you forget to include one?","commit_id":"d2d41f45e58fdc7d97fefd39cf0fd04c96d48aee"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9d15b10e76933e0cbcaeeebde5fe7ce2ddc0b6c","unresolved":false,"context_lines":[{"line_number":932,"context_line":""},{"line_number":933,"context_line":"    @property"},{"line_number":934,"context_line":"    def metadetails(self):"},{"line_number":935,"context_line":"        return dict([(m.key, m.value) for m in self._metadata])"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"class AgentBuild(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAEn%2F%2F5Ug%3D","line":935,"in_reply_to":"AAAAEn%2F%2F5Us%3D","updated":"2012-02-17 19:51:21.000000000","message":"It got merged a while ago.","commit_id":"d2d41f45e58fdc7d97fefd39cf0fd04c96d48aee"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"61813786fae91d2235d0aa07cac09c3197682837","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"              VolumeMetadata,"},{"line_number":1045,"context_line":"              VolumeTypeExtraSpecs,"},{"line_number":1046,"context_line":"              VolumeTypes,"},{"line_number":1047,"context_line":"              Zone,)"},{"line_number":1048,"context_line":"    engine \u003d create_engine(FLAGS.sql_connection, echo\u003dFalse)"},{"line_number":1049,"context_line":"    for model in models:"},{"line_number":1050,"context_line":"        model.metadata.create_all(engine)"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAEn%2F%2F5VA%3D","line":1047,"updated":"2012-02-17 19:49:15.000000000","message":"Minor nit: You could put the parenthesis on the next line.  I have to say, I *like* this refactoring :)","commit_id":"d2d41f45e58fdc7d97fefd39cf0fd04c96d48aee"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9d15b10e76933e0cbcaeeebde5fe7ce2ddc0b6c","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"              VolumeMetadata,"},{"line_number":1045,"context_line":"              VolumeTypeExtraSpecs,"},{"line_number":1046,"context_line":"              VolumeTypes,"},{"line_number":1047,"context_line":"              Zone,)"},{"line_number":1048,"context_line":"    engine \u003d create_engine(FLAGS.sql_connection, echo\u003dFalse)"},{"line_number":1049,"context_line":"    for model in models:"},{"line_number":1050,"context_line":"        model.metadata.create_all(engine)"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAEn%2F%2F5Uc%3D","line":1047,"in_reply_to":"AAAAEn%2F%2F5VA%3D","updated":"2012-02-17 19:51:21.000000000","message":"Done","commit_id":"d2d41f45e58fdc7d97fefd39cf0fd04c96d48aee"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"d4919da7159f811befbf8f4cf7159248ac4d2b08","unresolved":false,"context_lines":[{"line_number":939,"context_line":""},{"line_number":940,"context_line":"    @property"},{"line_number":941,"context_line":"    def metadetails(self):"},{"line_number":942,"context_line":"        return dict([(m.key, m.value) for m in self._metadata])"},{"line_number":943,"context_line":""},{"line_number":944,"context_line":""},{"line_number":945,"context_line":"class AgentBuild(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F5Ps%3D","line":942,"updated":"2012-02-17 20:31:38.000000000","message":"The migration got merged prior to the model being updated?  Which migration is it?","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"31ca14c3f067d55cbeb199295fb907ebeaec0de7","unresolved":false,"context_lines":[{"line_number":939,"context_line":""},{"line_number":940,"context_line":"    @property"},{"line_number":941,"context_line":"    def metadetails(self):"},{"line_number":942,"context_line":"        return dict([(m.key, m.value) for m in self._metadata])"},{"line_number":943,"context_line":""},{"line_number":944,"context_line":""},{"line_number":945,"context_line":"class AgentBuild(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F5KI%3D","line":942,"in_reply_to":"AAAAEn%2F%2F5LU%3D","updated":"2012-02-17 22:08:02.000000000","message":"OK, I think I see what\u0027s happening here…","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9f4bd5f3ac7c4305559edb474ba3e36c3e168ebe","unresolved":false,"context_lines":[{"line_number":939,"context_line":""},{"line_number":940,"context_line":"    @property"},{"line_number":941,"context_line":"    def metadetails(self):"},{"line_number":942,"context_line":"        return dict([(m.key, m.value) for m in self._metadata])"},{"line_number":943,"context_line":""},{"line_number":944,"context_line":""},{"line_number":945,"context_line":"class AgentBuild(BASE, NovaBase):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAEn%2F%2F5LU%3D","line":942,"in_reply_to":"AAAAEn%2F%2F5Ps%3D","updated":"2012-02-17 21:50:40.000000000","message":"the migration is 071_add_host_aggregate_tables.py","commit_id":"f1245208a6a05b96ab5473096e7d853036dd4a85"}],"nova/exception.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":920,"context_line":""},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"class AggregateError(NovaException):"},{"line_number":923,"context_line":"    message \u003d _(\"Aggregate %(aggregate_id)s: action \u0027%(action)s\u0027 \""},{"line_number":924,"context_line":"                \"caused an error: \") + \"%(reason)s.\""},{"line_number":925,"context_line":""},{"line_number":926,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bz4%3D","line":923,"updated":"2012-02-04 01:49:11.000000000","message":"Consider including %(reason)s in the text of the localizable message.  Otherwise, translations won\u0027t be able to change the position of the value for reason.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":920,"context_line":""},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"class AggregateError(NovaException):"},{"line_number":923,"context_line":"    message \u003d _(\"Aggregate %(aggregate_id)s: action \u0027%(action)s\u0027 \""},{"line_number":924,"context_line":"                \"caused an error: \") + \"%(reason)s.\""},{"line_number":925,"context_line":""},{"line_number":926,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BEY%3D","line":923,"in_reply_to":"AAAAEn%2F%2F%2Bz4%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"}],"nova/tests/test_compute.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":3066,"context_line":""},{"line_number":3067,"context_line":""},{"line_number":3068,"context_line":"class ComputeAPIAggrTestCase(test.TestCase):"},{"line_number":3069,"context_line":"    \"\"\"This for unit coverage of aggregate-related methods"},{"line_number":3070,"context_line":"    defined in in nova.compute.api.\"\"\""},{"line_number":3071,"context_line":""},{"line_number":3072,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bzw%3D","line":3069,"updated":"2012-02-04 01:49:11.000000000","message":"Grammar police!","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":3066,"context_line":""},{"line_number":3067,"context_line":""},{"line_number":3068,"context_line":"class ComputeAPIAggrTestCase(test.TestCase):"},{"line_number":3069,"context_line":"    \"\"\"This for unit coverage of aggregate-related methods"},{"line_number":3070,"context_line":"    defined in in nova.compute.api.\"\"\""},{"line_number":3071,"context_line":""},{"line_number":3072,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BEM%3D","line":3069,"in_reply_to":"AAAAEn%2F%2F%2Bzw%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":3067,"context_line":""},{"line_number":3068,"context_line":"class ComputeAPIAggrTestCase(test.TestCase):"},{"line_number":3069,"context_line":"    \"\"\"This for unit coverage of aggregate-related methods"},{"line_number":3070,"context_line":"    defined in in nova.compute.api.\"\"\""},{"line_number":3071,"context_line":""},{"line_number":3072,"context_line":"    def setUp(self):"},{"line_number":3073,"context_line":"        super(ComputeAPIAggrTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bzs%3D","line":3070,"updated":"2012-02-04 01:49:11.000000000","message":"Double \u0027in\u0027","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":3067,"context_line":""},{"line_number":3068,"context_line":"class ComputeAPIAggrTestCase(test.TestCase):"},{"line_number":3069,"context_line":"    \"\"\"This for unit coverage of aggregate-related methods"},{"line_number":3070,"context_line":"    defined in in nova.compute.api.\"\"\""},{"line_number":3071,"context_line":""},{"line_number":3072,"context_line":"    def setUp(self):"},{"line_number":3073,"context_line":"        super(ComputeAPIAggrTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BEI%3D","line":3070,"in_reply_to":"AAAAEn%2F%2F%2Bzs%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":3283,"context_line":"                          self.context, aggr[\u0027id\u0027], \u0027invalid_host\u0027)"},{"line_number":3284,"context_line":""},{"line_number":3285,"context_line":""},{"line_number":3286,"context_line":"class ComputeAggrTestCase(BaseTestCase):"},{"line_number":3287,"context_line":"    \"\"\"This for unit coverage of aggregate-related methods"},{"line_number":3288,"context_line":"    defined in nova.compute.manager.\"\"\""},{"line_number":3289,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bwo%3D","line":3286,"updated":"2012-02-04 01:49:11.000000000","message":"The two tests appear to be substantively similar, and factoring out the duplicated code might make for clearer tests.  \n\nIs there good reason not to test the unhappy path?","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":3283,"context_line":"                          self.context, aggr[\u0027id\u0027], \u0027invalid_host\u0027)"},{"line_number":3284,"context_line":""},{"line_number":3285,"context_line":""},{"line_number":3286,"context_line":"class ComputeAggrTestCase(BaseTestCase):"},{"line_number":3287,"context_line":"    \"\"\"This for unit coverage of aggregate-related methods"},{"line_number":3288,"context_line":"    defined in nova.compute.manager.\"\"\""},{"line_number":3289,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BEE%3D","line":3286,"in_reply_to":"AAAAEn%2F%2F%2Bwo%3D","updated":"2012-02-07 17:24:52.000000000","message":"They cover different parts of nova. One covers the AggregateAPI in nova.compute.api, the other covers nova.compute.manager.\n\nNo good reason not to test the unhappy path, just lack of time :)","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":3354,"context_line":"                       fake_driver_add_to_aggregate)"},{"line_number":3355,"context_line":""},{"line_number":3356,"context_line":"        def fake_db_aggregate_update(context, aggregate_id, values):"},{"line_number":3357,"context_line":"            fake_db_aggregate_update.called \u003d True"},{"line_number":3358,"context_line":"            self.assertEqual(values[\"foo\"], \"bar\", \"db update values\")"},{"line_number":3359,"context_line":"        self.stubs.Set(self.compute.db, \"aggregate_update\","},{"line_number":3360,"context_line":"                       fake_db_aggregate_update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F99k%3D","line":3357,"updated":"2012-02-07 20:11:21.000000000","message":"This flag is not being checked.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":3354,"context_line":"                       fake_driver_add_to_aggregate)"},{"line_number":3355,"context_line":""},{"line_number":3356,"context_line":"        def fake_db_aggregate_update(context, aggregate_id, values):"},{"line_number":3357,"context_line":"            fake_db_aggregate_update.called \u003d True"},{"line_number":3358,"context_line":"            self.assertEqual(values[\"foo\"], \"bar\", \"db update values\")"},{"line_number":3359,"context_line":"        self.stubs.Set(self.compute.db, \"aggregate_update\","},{"line_number":3360,"context_line":"                       fake_db_aggregate_update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F9aQ%3D","line":3357,"in_reply_to":"AAAAEn%2F%2F99k%3D","updated":"2012-02-08 17:14:44.000000000","message":"this turned out to be old and no longer used, well spotted :)","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":3371,"context_line":"                       fake_driver_remove_from_aggregate)"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        def fake_db_aggregate_update(context, aggregate_id, values):"},{"line_number":3374,"context_line":"            fake_db_aggregate_update.called \u003d True"},{"line_number":3375,"context_line":"            self.assertEqual(values[\"foo\"], \"bar\", \"db update values\")"},{"line_number":3376,"context_line":"        self.stubs.Set(self.compute.db, \"aggregate_update\","},{"line_number":3377,"context_line":"                       fake_db_aggregate_update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F99c%3D","line":3374,"updated":"2012-02-07 20:11:21.000000000","message":"This flag is not being checked.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":3371,"context_line":"                       fake_driver_remove_from_aggregate)"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        def fake_db_aggregate_update(context, aggregate_id, values):"},{"line_number":3374,"context_line":"            fake_db_aggregate_update.called \u003d True"},{"line_number":3375,"context_line":"            self.assertEqual(values[\"foo\"], \"bar\", \"db update values\")"},{"line_number":3376,"context_line":"        self.stubs.Set(self.compute.db, \"aggregate_update\","},{"line_number":3377,"context_line":"                       fake_db_aggregate_update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F9aM%3D","line":3374,"in_reply_to":"AAAAEn%2F%2F99c%3D","updated":"2012-02-08 17:14:44.000000000","message":"this turned out to be old and no longer used, well spotted :)","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"}],"nova/tests/test_db_api.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        expected \u003d {uuids[0]: [], uuids[1]: []}"},{"line_number":260,"context_line":"        self.assertEqual(expected, instance_faults)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"    def test_dns_registration(self):"},{"line_number":263,"context_line":"        domain1 \u003d \u0027test.domain.one\u0027"},{"line_number":264,"context_line":"        domain2 \u003d \u0027test.domain.two\u0027"},{"line_number":265,"context_line":"        testzone \u003d \u0027testzone\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Byw%3D","line":262,"updated":"2012-02-04 01:49:11.000000000","message":"Why is this unrelated test relocation included with the change?","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        expected \u003d {uuids[0]: [], uuids[1]: []}"},{"line_number":260,"context_line":"        self.assertEqual(expected, instance_faults)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"    def test_dns_registration(self):"},{"line_number":263,"context_line":"        domain1 \u003d \u0027test.domain.one\u0027"},{"line_number":264,"context_line":"        domain2 \u003d \u0027test.domain.two\u0027"},{"line_number":265,"context_line":"        testzone \u003d \u0027testzone\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BD0%3D","line":262,"in_reply_to":"AAAAEn%2F%2F%2Byw%3D","updated":"2012-02-07 17:24:52.000000000","message":"well, I found that it was in the wrong test case class.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"}],"nova/tests/test_xenapi.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":1743,"context_line":"        self.conn.add_to_aggregate(None, None, None)"},{"line_number":1744,"context_line":"        self.assertTrue(fake_add_to_aggregate.called)"},{"line_number":1745,"context_line":""},{"line_number":1746,"context_line":"    def test_add_master(self):"},{"line_number":1747,"context_line":"        \"\"\"Ensure that metadata are set correctly.\"\"\""},{"line_number":1748,"context_line":"        def fake_init_pool(id, name):"},{"line_number":1749,"context_line":"            fake_init_pool.called \u003d True"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BwY%3D","line":1746,"updated":"2012-02-04 01:49:11.000000000","message":"Consider using a more descriptive test name instead of a non-descriptive test name and a comment: \n\ntest_add_to_aggregate_for_first_host_sets_metadata","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":1743,"context_line":"        self.conn.add_to_aggregate(None, None, None)"},{"line_number":1744,"context_line":"        self.assertTrue(fake_add_to_aggregate.called)"},{"line_number":1745,"context_line":""},{"line_number":1746,"context_line":"    def test_add_master(self):"},{"line_number":1747,"context_line":"        \"\"\"Ensure that metadata are set correctly.\"\"\""},{"line_number":1748,"context_line":"        def fake_init_pool(id, name):"},{"line_number":1749,"context_line":"            fake_init_pool.called \u003d True"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BDc%3D","line":1746,"in_reply_to":"AAAAEn%2F%2F%2BwY%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"        self.stubs.Set(self.conn._pool, \"_join_slave\", fake_join_slave)"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        self.flags(host\u003d\u0027fake_host\u0027)"},{"line_number":1771,"context_line":"        metadata \u003d {\u0027master_password\u0027: \u0027fake_pass\u0027,"},{"line_number":1772,"context_line":"                    \u0027master_compute\u0027: \u0027fake_host\u0027,"},{"line_number":1773,"context_line":"                    \u0027master_address\u0027: \u0027127.0.0.1\u0027,"},{"line_number":1774,"context_line":"                    \u0027master_username\u0027: \u0027fake_root\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bvw%3D","line":1771,"updated":"2012-02-04 01:49:11.000000000","message":"DRY - consider defining common metadata in setUp.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"        self.stubs.Set(self.conn._pool, \"_join_slave\", fake_join_slave)"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"        self.flags(host\u003d\u0027fake_host\u0027)"},{"line_number":1771,"context_line":"        metadata \u003d {\u0027master_password\u0027: \u0027fake_pass\u0027,"},{"line_number":1772,"context_line":"                    \u0027master_compute\u0027: \u0027fake_host\u0027,"},{"line_number":1773,"context_line":"                    \u0027master_address\u0027: \u0027127.0.0.1\u0027,"},{"line_number":1774,"context_line":"                    \u0027master_username\u0027: \u0027fake_root\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BDg%3D","line":1771,"in_reply_to":"AAAAEn%2F%2F%2Bvw%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":1855,"context_line":"        self.assertDictMatch({}, result.metadetails)"},{"line_number":1856,"context_line":"        self.assertEqual(aggregate_states.ACTIVE, result.operational_state)"},{"line_number":1857,"context_line":""},{"line_number":1858,"context_line":"    def test_remote_master_non_empty(self):"},{"line_number":1859,"context_line":"        \"\"\"Ensure AggregateError is raised if removing the master.\"\"\""},{"line_number":1860,"context_line":"        def fake_clear_pool_raise(id):"},{"line_number":1861,"context_line":"            raise exception.AggregateError()"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BwA%3D","line":1858,"updated":"2012-02-04 01:49:11.000000000","message":"This test is only validating that _clear_pool is being called, and this code path is already being tested by test_remove_master_solo.  Depending on the intent, this test should either be rewritten or removed.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":1855,"context_line":"        self.assertDictMatch({}, result.metadetails)"},{"line_number":1856,"context_line":"        self.assertEqual(aggregate_states.ACTIVE, result.operational_state)"},{"line_number":1857,"context_line":""},{"line_number":1858,"context_line":"    def test_remote_master_non_empty(self):"},{"line_number":1859,"context_line":"        \"\"\"Ensure AggregateError is raised if removing the master.\"\"\""},{"line_number":1860,"context_line":"        def fake_clear_pool_raise(id):"},{"line_number":1861,"context_line":"            raise exception.AggregateError()"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BDU%3D","line":1858,"in_reply_to":"AAAAEn%2F%2F%2BwA%3D","updated":"2012-02-07 17:24:52.000000000","message":"this should also cover the fact that an exception is raised, so I am not sure it\u0027s a complete write-off","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"        self.assertDictMatch({}, result.metadetails)"},{"line_number":1838,"context_line":"        self.assertEqual(aggregate_states.ACTIVE, result.operational_state)"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"    def test_remote_master_non_empty(self):"},{"line_number":1841,"context_line":"        \"\"\"Ensure AggregateError is raised if removing the master.\"\"\""},{"line_number":1842,"context_line":"        def fake_clear_pool_raise(id):"},{"line_number":1843,"context_line":"            raise exception.AggregateError()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F97M%3D","line":1840,"updated":"2012-02-07 20:11:21.000000000","message":"Should I assume that this test is intended to verify that remove_from_aggregate passes AggregateError through to the caller?  Otherwise, test_remove_master_solo is already ensuring that _clear_pool is being called.  Whatever fake_clear_pool does - whether indicating it has been called or raising an exception - the effective coverage is the same.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"        self.assertDictMatch({}, result.metadetails)"},{"line_number":1838,"context_line":"        self.assertEqual(aggregate_states.ACTIVE, result.operational_state)"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"    def test_remote_master_non_empty(self):"},{"line_number":1841,"context_line":"        \"\"\"Ensure AggregateError is raised if removing the master.\"\"\""},{"line_number":1842,"context_line":"        def fake_clear_pool_raise(id):"},{"line_number":1843,"context_line":"            raise exception.AggregateError()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F9aI%3D","line":1840,"in_reply_to":"AAAAEn%2F%2F97M%3D","updated":"2012-02-08 17:14:44.000000000","message":"I believe I got a few things mixed up here, let me review. Thanks for drawing my attention to this.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"}],"nova/virt/xenapi/fake.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"63cea0763ac55a738164688f9bc8326c68d14929","unresolved":false,"context_lines":[{"line_number":450,"context_line":""},{"line_number":451,"context_line":"    def VDI_remove_from_other_config(self, _1, vdi_ref, key):"},{"line_number":452,"context_line":"        db_ref \u003d _db_content[\u0027VDI\u0027][vdi_ref]"},{"line_number":453,"context_line":"        if not \u0027other_config\u0027 in db_ref:"},{"line_number":454,"context_line":"            return"},{"line_number":455,"context_line":"        db_ref[\u0027other_config\u0027][key] \u003d None"},{"line_number":456,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6uU%3D","line":453,"updated":"2012-02-14 22:36:48.000000000","message":"Consider using a positive test and an implicit return.\n\nif \u0027other_config\u0027 in db_ref:\n  db_ref[\u0027other_config\u0027][key] \u003d None","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3eae67bad935d44c0b50f30f3c660660f96c56a0","unresolved":false,"context_lines":[{"line_number":450,"context_line":""},{"line_number":451,"context_line":"    def VDI_remove_from_other_config(self, _1, vdi_ref, key):"},{"line_number":452,"context_line":"        db_ref \u003d _db_content[\u0027VDI\u0027][vdi_ref]"},{"line_number":453,"context_line":"        if not \u0027other_config\u0027 in db_ref:"},{"line_number":454,"context_line":"            return"},{"line_number":455,"context_line":"        db_ref[\u0027other_config\u0027][key] \u003d None"},{"line_number":456,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6rM%3D","line":453,"in_reply_to":"AAAAEn%2F%2F6uU%3D","updated":"2012-02-14 22:51:17.000000000","message":"I haven\u0027t touched this code, are you sure you mean this?","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"63cea0763ac55a738164688f9bc8326c68d14929","unresolved":false,"context_lines":[{"line_number":456,"context_line":""},{"line_number":457,"context_line":"    def VDI_add_to_other_config(self, _1, vdi_ref, key, value):"},{"line_number":458,"context_line":"        db_ref \u003d _db_content[\u0027VDI\u0027][vdi_ref]"},{"line_number":459,"context_line":"        if not \u0027other_config\u0027 in db_ref:"},{"line_number":460,"context_line":"            db_ref[\u0027other_config\u0027] \u003d {}"},{"line_number":461,"context_line":"        db_ref[\u0027other_config\u0027][key] \u003d value"},{"line_number":462,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6uI%3D","line":459,"updated":"2012-02-14 22:36:48.000000000","message":"Consider using setdefault() instead.","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3eae67bad935d44c0b50f30f3c660660f96c56a0","unresolved":false,"context_lines":[{"line_number":456,"context_line":""},{"line_number":457,"context_line":"    def VDI_add_to_other_config(self, _1, vdi_ref, key, value):"},{"line_number":458,"context_line":"        db_ref \u003d _db_content[\u0027VDI\u0027][vdi_ref]"},{"line_number":459,"context_line":"        if not \u0027other_config\u0027 in db_ref:"},{"line_number":460,"context_line":"            db_ref[\u0027other_config\u0027] \u003d {}"},{"line_number":461,"context_line":"        db_ref[\u0027other_config\u0027][key] \u003d value"},{"line_number":462,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAEn%2F%2F6rE%3D","line":459,"in_reply_to":"AAAAEn%2F%2F6uI%3D","updated":"2012-02-14 22:51:17.000000000","message":"ditto","commit_id":"ac2c63813a1c6a78809543ffd543fd6a49eeb96a"}],"nova/virt/xenapi/pool.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"xenapi_pool_opts \u003d ["},{"line_number":36,"context_line":"    cfg.BoolOpt(\u0027use_join_force\u0027,"},{"line_number":37,"context_line":"               default\u003dTrue,"},{"line_number":38,"context_line":"               help\u003d\u0027To use for hosts with different CPUs\u0027),"},{"line_number":39,"context_line":"    ]"},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Bx0%3D","line":37,"updated":"2012-02-04 01:49:11.000000000","message":"PEP8 vertical alignment","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"xenapi_pool_opts \u003d ["},{"line_number":36,"context_line":"    cfg.BoolOpt(\u0027use_join_force\u0027,"},{"line_number":37,"context_line":"               default\u003dTrue,"},{"line_number":38,"context_line":"               help\u003d\u0027To use for hosts with different CPUs\u0027),"},{"line_number":39,"context_line":"    ]"},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BDA%3D","line":37,"in_reply_to":"AAAAEn%2F%2F%2Bx0%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # the pool is already up and running, we need to figure out"},{"line_number":77,"context_line":"            # whether we can serve the request from this host or not."},{"line_number":78,"context_line":"            master_compute \u003d aggregate.metadetails[\u0027master_compute\u0027]"},{"line_number":79,"context_line":"            if master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host:"},{"line_number":80,"context_line":"                # this is the master -\u003e  do a pool-join"},{"line_number":81,"context_line":"                # To this aim, nova compute on the slave has to go down."},{"line_number":82,"context_line":"                # NOTE: it is assumed that ONLY nova compute is running now"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2Byg%3D","line":79,"updated":"2012-02-04 01:49:11.000000000","message":"Consider simplifying the logic of this if/else construct:\n\nif master_compute \u003d\u003d host or not master_compute:\n  return\n\nif master_compute \u003d\u003d FLAGS.host:\n  pass  \nelse:\n  pass\n\nAlso, consider making conditional tests self-documenting in preference to documenting them explicitly, e.g.:\n\non_master \u003d master_compute \u003d\u003d FLAGS.host\nif on_master:\n  pass","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # the pool is already up and running, we need to figure out"},{"line_number":77,"context_line":"            # whether we can serve the request from this host or not."},{"line_number":78,"context_line":"            master_compute \u003d aggregate.metadetails[\u0027master_compute\u0027]"},{"line_number":79,"context_line":"            if master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host:"},{"line_number":80,"context_line":"                # this is the master -\u003e  do a pool-join"},{"line_number":81,"context_line":"                # To this aim, nova compute on the slave has to go down."},{"line_number":82,"context_line":"                # NOTE: it is assumed that ONLY nova compute is running now"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BCY%3D","line":79,"in_reply_to":"AAAAEn%2F%2F%2Byg%3D","updated":"2012-02-07 17:24:52.000000000","message":"I am not sure I see the gain in doing what you suggest in the first part of this comment. Can you elaborate a bit more?\n\nThanks!","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def remove_from_aggregate(self, context, aggregate, host, **kwargs):"},{"line_number":94,"context_line":"        master_compute \u003d aggregate.metadetails.get(\u0027master_compute\u0027)"},{"line_number":95,"context_line":"        if master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host:"},{"line_number":96,"context_line":"            # this is the master -\u003e instruct it to eject a host from the pool"},{"line_number":97,"context_line":"            self._eject_slave(aggregate.id)"},{"line_number":98,"context_line":"        elif master_compute \u003d\u003d host:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2ByY%3D","line":95,"updated":"2012-02-04 01:49:11.000000000","message":"Consider simplifying this if/else construct:\n\nif master_compute \u003d\u003d host:\n  pass\nelif master_compute \u003d\u003d FLAGS.host:\n  pass\nelif master_compute:\n  pass\nelse:\n  raise","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def remove_from_aggregate(self, context, aggregate, host, **kwargs):"},{"line_number":94,"context_line":"        master_compute \u003d aggregate.metadetails.get(\u0027master_compute\u0027)"},{"line_number":95,"context_line":"        if master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host:"},{"line_number":96,"context_line":"            # this is the master -\u003e instruct it to eject a host from the pool"},{"line_number":97,"context_line":"            self._eject_slave(aggregate.id)"},{"line_number":98,"context_line":"        elif master_compute \u003d\u003d host:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BCU%3D","line":95,"in_reply_to":"AAAAEn%2F%2F%2ByY%3D","updated":"2012-02-07 17:24:52.000000000","message":"ditto","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        session \u003d self.XenAPI.Session(url)"},{"line_number":124,"context_line":"        session.login_with_password(user, password)"},{"line_number":125,"context_line":"        try:"},{"line_number":126,"context_line":"            pool_join \u003d FLAGS.use_join_force and \\"},{"line_number":127,"context_line":"                        session.xenapi.join_force or \\"},{"line_number":128,"context_line":"                        session.xenapi.join"},{"line_number":129,"context_line":"            pool_join(FLAGS.xenapi_connection_username,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2ByM%3D","line":126,"updated":"2012-02-04 01:49:11.000000000","message":"PEP8: Prefer parenthesis to backslashes for line continuation.","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        session \u003d self.XenAPI.Session(url)"},{"line_number":124,"context_line":"        session.login_with_password(user, password)"},{"line_number":125,"context_line":"        try:"},{"line_number":126,"context_line":"            pool_join \u003d FLAGS.use_join_force and \\"},{"line_number":127,"context_line":"                        session.xenapi.join_force or \\"},{"line_number":128,"context_line":"                        session.xenapi.join"},{"line_number":129,"context_line":"            pool_join(FLAGS.xenapi_connection_username,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BCw%3D","line":126,"in_reply_to":"AAAAEn%2F%2F%2ByM%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"339caf128686d40341d75df7a2330a0fbad5d2cb","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            LOG.error(_(\"Pool-Join failed: %(e)s\") % locals())"},{"line_number":133,"context_line":"            raise exception.AggregateError(aggregate_id\u003daggregate_id,"},{"line_number":134,"context_line":"                                           action\u003d\u0027add_to_aggregate\u0027,"},{"line_number":135,"context_line":"                                           reason\u003d\u0027Unable to join %(host)s \u0027"},{"line_number":136,"context_line":"                                                  \u0027in the pool\u0027 % locals())"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _eject_slave(self, aggregate_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2ByI%3D","line":135,"updated":"2012-02-04 01:49:11.000000000","message":"Should the text of the reason be localizable?","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d2fc1a25cbc3ae70507dad23ab2cf178592b05f9","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            LOG.error(_(\"Pool-Join failed: %(e)s\") % locals())"},{"line_number":133,"context_line":"            raise exception.AggregateError(aggregate_id\u003daggregate_id,"},{"line_number":134,"context_line":"                                           action\u003d\u0027add_to_aggregate\u0027,"},{"line_number":135,"context_line":"                                           reason\u003d\u0027Unable to join %(host)s \u0027"},{"line_number":136,"context_line":"                                                  \u0027in the pool\u0027 % locals())"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _eject_slave(self, aggregate_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAEn%2F%2F%2BCk%3D","line":135,"in_reply_to":"AAAAEn%2F%2F%2ByI%3D","updated":"2012-02-07 17:24:52.000000000","message":"Done","commit_id":"070677a182648dc448b41cd38067423a22f4d195"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            # the pool is already up and running, we need to figure out"},{"line_number":78,"context_line":"            # whether we can serve the request from this host or not."},{"line_number":79,"context_line":"            master_compute \u003d aggregate.metadetails[\u0027master_compute\u0027]"},{"line_number":80,"context_line":"            if master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host:"},{"line_number":81,"context_line":"                # this is the master -\u003e  do a pool-join"},{"line_number":82,"context_line":"                # To this aim, nova compute on the slave has to go down."},{"line_number":83,"context_line":"                # NOTE: it is assumed that ONLY nova compute is running now"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F97U%3D","line":80,"updated":"2012-02-07 20:11:21.000000000","message":"My suggestion for simplifying the logic of the if/else conditionals would enhance the readability of the code, but there is no requirement that you act on it.  \n\nI do strongly encourage the use of self-documenting conditionals where possible, though, to maximize readability:\n\nis_master \u003d (master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host)\nif is_master:\n pass\n\nThis code clearly documents it\u0027s intent and does not require  comments.  If intent can be defined in code alone, it\u0027s that much easier to maintain - when change is required, only code has to be updated.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            # the pool is already up and running, we need to figure out"},{"line_number":78,"context_line":"            # whether we can serve the request from this host or not."},{"line_number":79,"context_line":"            master_compute \u003d aggregate.metadetails[\u0027master_compute\u0027]"},{"line_number":80,"context_line":"            if master_compute \u003d\u003d FLAGS.host and master_compute !\u003d host:"},{"line_number":81,"context_line":"                # this is the master -\u003e  do a pool-join"},{"line_number":82,"context_line":"                # To this aim, nova compute on the slave has to go down."},{"line_number":83,"context_line":"                # NOTE: it is assumed that ONLY nova compute is running now"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAEn%2F%2F9bc%3D","line":80,"in_reply_to":"AAAAEn%2F%2F97U%3D","updated":"2012-02-08 17:14:44.000000000","message":"In cases where the logical expression is fairly structured, yes I totally agree with you. In this particular case...I am not sure we gain much.\n\nThe extra comments are to further explain what\u0027s going on, but I believe the code is self-explanatory.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            task \u003d self._session.async_call_plugin(\u0027xenhost\u0027,"},{"line_number":141,"context_line":"                                                   \u0027host_join\u0027, args)"},{"line_number":142,"context_line":"            self._session.wait_for_task(task)"},{"line_number":143,"context_line":"        except self.XenAPI.Failure, e:"},{"line_number":144,"context_line":"            LOG.error(_(\"Pool-Join failed: %(e)s\") % locals())"},{"line_number":145,"context_line":"            raise exception.AggregateError(aggregate_id\u003daggregate_id,"},{"line_number":146,"context_line":"                                           action\u003d\u0027add_to_aggregate\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F89w%3D","line":143,"updated":"2012-02-09 00:32:28.000000000","message":"Is this intended to run on 2.4?  If not, consider using the \u0027Exception as e\u0027 syntax.  It was pointed out to me in a recent code review this is the preferred syntax in openstack.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            task \u003d self._session.async_call_plugin(\u0027xenhost\u0027,"},{"line_number":141,"context_line":"                                                   \u0027host_join\u0027, args)"},{"line_number":142,"context_line":"            self._session.wait_for_task(task)"},{"line_number":143,"context_line":"        except self.XenAPI.Failure, e:"},{"line_number":144,"context_line":"            LOG.error(_(\"Pool-Join failed: %(e)s\") % locals())"},{"line_number":145,"context_line":"            raise exception.AggregateError(aggregate_id\u003daggregate_id,"},{"line_number":146,"context_line":"                                           action\u003d\u0027add_to_aggregate\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F8uI%3D","line":143,"in_reply_to":"AAAAEn%2F%2F89w%3D","updated":"2012-02-09 16:57:04.000000000","message":"no, here it\u0027s assumed we have 2.6 or above. Will do.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            LOG.error(_(\"Pool-set_name_label failed: %(e)s\") % locals())"},{"line_number":187,"context_line":"            raise exception.AggregateError(aggregate_id\u003daggregate_id,"},{"line_number":188,"context_line":"                                           action\u003d\u0027remove_from_aggregate\u0027,"},{"line_number":189,"context_line":"                                           reason\u003dstr(e.details))"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"def forward_request(context, request_type, master, aggregate_id,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F89s%3D","line":189,"updated":"2012-02-09 00:32:28.000000000","message":"I see this conversion to a string everywhere, so I\u0027m going to assume that e.details will never result in non-ascii characters.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            LOG.error(_(\"Pool-set_name_label failed: %(e)s\") % locals())"},{"line_number":187,"context_line":"            raise exception.AggregateError(aggregate_id\u003daggregate_id,"},{"line_number":188,"context_line":"                                           action\u003d\u0027remove_from_aggregate\u0027,"},{"line_number":189,"context_line":"                                           reason\u003dstr(e.details))"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"def forward_request(context, request_type, master, aggregate_id,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAEn%2F%2F8uA%3D","line":189,"in_reply_to":"AAAAEn%2F%2F89s%3D","updated":"2012-02-09 16:57:04.000000000","message":"this is a XenAPI.Failure: so a dictionary with SOMETHING_WENT_WRONG message plus some other bits. so only ascii chararacters AFAIK","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    \"\"\"Join a remote host into a pool whose master is the host"},{"line_number":264,"context_line":"    where the plugin is called from. The following constraints apply:"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    - The host must have no VMs running, except nova-compute, which will be "},{"line_number":267,"context_line":"      shut down (and restarted upon pool-join) automatically,"},{"line_number":268,"context_line":"    - The host must have no shared storage currently set up,"},{"line_number":269,"context_line":"    - The host must have the same license of the master,"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F99s%3D","line":266,"updated":"2012-02-07 20:11:21.000000000","message":"Whitespace","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    \"\"\"Join a remote host into a pool whose master is the host"},{"line_number":264,"context_line":"    where the plugin is called from. The following constraints apply:"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    - The host must have no VMs running, except nova-compute, which will be "},{"line_number":267,"context_line":"      shut down (and restarted upon pool-join) automatically,"},{"line_number":268,"context_line":"    - The host must have no shared storage currently set up,"},{"line_number":269,"context_line":"    - The host must have the same license of the master,"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9bQ%3D","line":266,"in_reply_to":"AAAAEn%2F%2F99s%3D","updated":"2012-02-08 17:14:44.000000000","message":"Done","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    - The host must have no shared storage currently set up,"},{"line_number":269,"context_line":"    - The host must have the same license of the master,"},{"line_number":270,"context_line":"    - The host must have the same supplemental packs as the master.\"\"\""},{"line_number":271,"context_line":"    def resume_compute(session, compute_ref, compute_uuid):"},{"line_number":272,"context_line":"        \"\"\"Resume compute node on slave host after pool join. This has to"},{"line_number":273,"context_line":"        happen regardless of the success or failure of the join operation.\"\"\""},{"line_number":274,"context_line":"        try:"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F97g%3D","line":271,"updated":"2012-02-07 20:11:21.000000000","message":"Consider defining this method at top-level rather than inline, as there is no way to target an inline function for testing.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":268,"context_line":"    - The host must have no shared storage currently set up,"},{"line_number":269,"context_line":"    - The host must have the same license of the master,"},{"line_number":270,"context_line":"    - The host must have the same supplemental packs as the master.\"\"\""},{"line_number":271,"context_line":"    def resume_compute(session, compute_ref, compute_uuid):"},{"line_number":272,"context_line":"        \"\"\"Resume compute node on slave host after pool join. This has to"},{"line_number":273,"context_line":"        happen regardless of the success or failure of the join operation.\"\"\""},{"line_number":274,"context_line":"        try:"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9bM%3D","line":271,"in_reply_to":"AAAAEn%2F%2F97g%3D","updated":"2012-02-08 17:14:44.000000000","message":"Done","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":277,"context_line":"        except XenAPI.Failure, e:"},{"line_number":278,"context_line":"            # if session is invalid, e.g. xapi has restarted, then the pool"},{"line_number":279,"context_line":"            # join has been successful, wait for xapi to become alive again"},{"line_number":280,"context_line":"            for c in xrange(1, 12):"},{"line_number":281,"context_line":"                try:"},{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F97Y%3D","line":280,"updated":"2012-02-07 20:11:21.000000000","message":"Consider making the timeout duration configurable via the method signature, or defined in a constant, since it is likely to change over time.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":277,"context_line":"        except XenAPI.Failure, e:"},{"line_number":278,"context_line":"            # if session is invalid, e.g. xapi has restarted, then the pool"},{"line_number":279,"context_line":"            # join has been successful, wait for xapi to become alive again"},{"line_number":280,"context_line":"            for c in xrange(1, 12):"},{"line_number":281,"context_line":"                try:"},{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9bI%3D","line":280,"in_reply_to":"AAAAEn%2F%2F97Y%3D","updated":"2012-02-08 17:14:44.000000000","message":"Done","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":279,"context_line":"            # join has been successful, wait for xapi to become alive again"},{"line_number":280,"context_line":"            for c in xrange(1, 12):"},{"line_number":281,"context_line":"                try:"},{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"},{"line_number":284,"context_line":"                    f.close()"},{"line_number":285,"context_line":"                    os.kill(xapi_pid, 0)"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F98Y%3D","line":282,"updated":"2012-02-07 20:11:21.000000000","message":"Consider using python\u0027s \u0027with\u0027 keyword in preference to an explicit close().","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":279,"context_line":"            # join has been successful, wait for xapi to become alive again"},{"line_number":280,"context_line":"            for c in xrange(1, 12):"},{"line_number":281,"context_line":"                try:"},{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"},{"line_number":284,"context_line":"                    f.close()"},{"line_number":285,"context_line":"                    os.kill(xapi_pid, 0)"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9bE%3D","line":282,"in_reply_to":"AAAAEn%2F%2F98Y%3D","updated":"2012-02-08 17:14:44.000000000","message":"I wish I could do that, but that\u0027s dom0, we\u0027re stuck with python 2.4 for now :(","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            for c in xrange(1, 12):"},{"line_number":281,"context_line":"                try:"},{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"},{"line_number":284,"context_line":"                    f.close()"},{"line_number":285,"context_line":"                    os.kill(xapi_pid, 0)"},{"line_number":286,"context_line":"                    cmd \u003d \"xe vm-start uuid\u003d%s\" % arg_dict.get(\u0027compute_uuid\u0027)"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F978%3D","line":283,"updated":"2012-02-07 20:11:21.000000000","message":"Why is the file variable being assigned the pid?  This assigns an integer to f, and the subsequent call to f.close() is guaranteed to raise an AttributeError.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            for c in xrange(1, 12):"},{"line_number":281,"context_line":"                try:"},{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"},{"line_number":284,"context_line":"                    f.close()"},{"line_number":285,"context_line":"                    os.kill(xapi_pid, 0)"},{"line_number":286,"context_line":"                    cmd \u003d \"xe vm-start uuid\u003d%s\" % arg_dict.get(\u0027compute_uuid\u0027)"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9bA%3D","line":283,"in_reply_to":"AAAAEn%2F%2F978%3D","updated":"2012-02-08 17:14:44.000000000","message":"this is a mistake, and I don\u0027t understand how it got there. Neverthless I\u0027ll remove the whole thing as I was getting the wrong end of the stick: it\u0027s not xapi on the master dom0 that\u0027s restarting...but the slave\u0027s.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"},{"line_number":284,"context_line":"                    f.close()"},{"line_number":285,"context_line":"                    os.kill(xapi_pid, 0)"},{"line_number":286,"context_line":"                    cmd \u003d \"xe vm-start uuid\u003d%s\" % arg_dict.get(\u0027compute_uuid\u0027)"},{"line_number":287,"context_line":"                    _run_command(cmd)"},{"line_number":288,"context_line":"                    return"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F98E%3D","line":285,"updated":"2012-02-07 20:11:21.000000000","message":"I\u0027m a bit confused - why is signal zero being sent?  To my knowledge this invokes error handling on the part of kill, but does not actually send anything to the process.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                    f \u003d open(\u0027/var/run/xapi.pid\u0027)"},{"line_number":283,"context_line":"                    f \u003d xapi_pid \u003d int(f.read())"},{"line_number":284,"context_line":"                    f.close()"},{"line_number":285,"context_line":"                    os.kill(xapi_pid, 0)"},{"line_number":286,"context_line":"                    cmd \u003d \"xe vm-start uuid\u003d%s\" % arg_dict.get(\u0027compute_uuid\u0027)"},{"line_number":287,"context_line":"                    _run_command(cmd)"},{"line_number":288,"context_line":"                    return"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9a8%3D","line":285,"in_reply_to":"AAAAEn%2F%2F98E%3D","updated":"2012-02-08 17:14:44.000000000","message":"this was a dumb way of poking for a process that is alive and kicking.","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"61e4818cbd8fc5a1c1bc95778238d6611aa34ced","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                    cmd \u003d \"xe vm-start uuid\u003d%s\" % arg_dict.get(\u0027compute_uuid\u0027)"},{"line_number":287,"context_line":"                    _run_command(cmd)"},{"line_number":288,"context_line":"                    return"},{"line_number":289,"context_line":"                except:"},{"line_number":290,"context_line":"                    time.sleep(10)"},{"line_number":291,"context_line":"        raise pluginlib.PluginError(\u0027Unrecoverable error!\u0027)"},{"line_number":292,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F98c%3D","line":289,"updated":"2012-02-07 20:11:21.000000000","message":"Consider trapping exceptions in a targeted fashion - catch-all handlers can hide implementation bugs (and there are bugs in the current implementation).","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6ca5249afeb190d1199aa7d12b80a1ffb688adbd","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                    cmd \u003d \"xe vm-start uuid\u003d%s\" % arg_dict.get(\u0027compute_uuid\u0027)"},{"line_number":287,"context_line":"                    _run_command(cmd)"},{"line_number":288,"context_line":"                    return"},{"line_number":289,"context_line":"                except:"},{"line_number":290,"context_line":"                    time.sleep(10)"},{"line_number":291,"context_line":"        raise pluginlib.PluginError(\u0027Unrecoverable error!\u0027)"},{"line_number":292,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAEn%2F%2F9a4%3D","line":289,"in_reply_to":"AAAAEn%2F%2F98c%3D","updated":"2012-02-08 17:14:44.000000000","message":"Done. I was in a rush :)","commit_id":"257be6f940b5744e309942a5a7631db5aa9a9a97"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":43,"context_line":"host_data_pattern \u003d re.compile(r\"\\s*(\\S+) \\([^\\)]+\\) *: ?(.*)\")"},{"line_number":44,"context_line":"config_file_path \u003d \"/usr/etc/xenhost.conf\""},{"line_number":45,"context_line":"DEFAULT_TRIES \u003d 24"},{"line_number":46,"context_line":"DEFAULT_SLEEP \u003d 10 "},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def jsonify(fnc):"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAEn%2F%2F9O4%3D","line":46,"updated":"2012-02-09 00:32:28.000000000","message":"Whitespace","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":43,"context_line":"host_data_pattern \u003d re.compile(r\"\\s*(\\S+) \\([^\\)]+\\) *: ?(.*)\")"},{"line_number":44,"context_line":"config_file_path \u003d \"/usr/etc/xenhost.conf\""},{"line_number":45,"context_line":"DEFAULT_TRIES \u003d 24"},{"line_number":46,"context_line":"DEFAULT_SLEEP \u003d 10 "},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def jsonify(fnc):"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAEn%2F%2F8t8%3D","line":46,"in_reply_to":"AAAAEn%2F%2F9O4%3D","updated":"2012-02-09 16:57:04.000000000","message":"doh! I filed a fix so that we don\u0027t stumble on this ever again :)","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    except XenAPI.Failure, e:"},{"line_number":110,"context_line":"        # if session is invalid, e.g. xapi has restarted, then the pool"},{"line_number":111,"context_line":"        # join has been successful, wait for xapi to become alive again"},{"line_number":112,"context_line":"        for c in xrange(1, DEFAULT_TRIES):"},{"line_number":113,"context_line":"            try:"},{"line_number":114,"context_line":"                _run_command(\"xe vm-start uuid\u003d%s\" % compute_uuid)"},{"line_number":115,"context_line":"                return"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAEn%2F%2F8%2BY%3D","line":112,"updated":"2012-02-09 00:32:28.000000000","message":"Consider using an overall timeout (i.e. 60s) rather than configuring the number of tries.  It also might be helpful to keep track of the total elapsed time to include in the error message if the slave does not become available.  Suggestions only, though, this implementation if fine.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    except XenAPI.Failure, e:"},{"line_number":110,"context_line":"        # if session is invalid, e.g. xapi has restarted, then the pool"},{"line_number":111,"context_line":"        # join has been successful, wait for xapi to become alive again"},{"line_number":112,"context_line":"        for c in xrange(1, DEFAULT_TRIES):"},{"line_number":113,"context_line":"            try:"},{"line_number":114,"context_line":"                _run_command(\"xe vm-start uuid\u003d%s\" % compute_uuid)"},{"line_number":115,"context_line":"                return"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAEn%2F%2F8t0%3D","line":112,"in_reply_to":"AAAAEn%2F%2F8%2BY%3D","updated":"2012-02-09 16:57:04.000000000","message":"can do.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"404097358e59c370154e5ef8ac722a4e9e749cc0","unresolved":false,"context_lines":[{"line_number":116,"context_line":"            except pluginlib.PluginError, e:"},{"line_number":117,"context_line":"                logging.exception(\u0027Waiting for slave to become available.\u0027)"},{"line_number":118,"context_line":"                time.sleep(DEFAULT_SLEEP)"},{"line_number":119,"context_line":"    raise pluginlib.PluginError(\u0027Unrecoverable error!\u0027)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"def _get_host_uuid():"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAEn%2F%2F8%2Bg%3D","line":119,"updated":"2012-02-09 00:32:28.000000000","message":"Should this exception be localizable, and perhaps more detailed (indicating that the slave didn\u0027t become available in the timeout duration)?","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afa65bfeacbe32acbd906b3c87779eb27c10dd7f","unresolved":false,"context_lines":[{"line_number":116,"context_line":"            except pluginlib.PluginError, e:"},{"line_number":117,"context_line":"                logging.exception(\u0027Waiting for slave to become available.\u0027)"},{"line_number":118,"context_line":"                time.sleep(DEFAULT_SLEEP)"},{"line_number":119,"context_line":"    raise pluginlib.PluginError(\u0027Unrecoverable error!\u0027)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"def _get_host_uuid():"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAAEn%2F%2F8t4%3D","line":119,"in_reply_to":"AAAAEn%2F%2F8%2Bg%3D","updated":"2012-02-09 16:57:04.000000000","message":"We are in xenserverland, it\u0027s my knowledge that these are not localized. I can tweak this a bit.","commit_id":"ff237e9ac50e30f1e9512287b11f3e8012887002"}]}