)]}'
{"nodepool/fakeprovider.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"c2db6d0e55669269f0292188d254b4390998b8ca","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        if hasattr(obj, \u0027id\u0027):"},{"line_number":58,"context_line":"            self._list.remove(obj)"},{"line_number":59,"context_line":"        else:"},{"line_number":60,"context_line":"            self._list.remove(self.get(obj))"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def create(self, **kw):"},{"line_number":63,"context_line":"        s \u003d Dummy(id\u003duuid.uuid4().hex,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAARn%2F%2F3AM%3D","line":60,"updated":"2013-08-21 02:58:43.000000000","message":"FWIW I\u0027d usually write this something like:\nif not safe_hasattr(obj, \u0027id\u0027):\n    obj \u003d self.get(obj)\nself._list.remove(obj)\n\nwhich is both less code and also fixes a bug (hasattr before 3.2 can swallow exceptions in appropriately).\n\nHowever - this is just really just cosmetic.","commit_id":"8dc6c870f26823c5163e8e613f6e76726b775fdd"}],"nodepool/nodepool.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fc99e6de5d399007a94a5a03808409a3f0c2ce52","unresolved":false,"context_lines":[{"line_number":525,"context_line":"                    stop_managers.append(oldmanager)"},{"line_number":526,"context_line":"                    oldmanager \u003d None"},{"line_number":527,"context_line":"            if oldmanager:"},{"line_number":528,"context_line":"                newconfig.managers[p.name] \u003d oldmanager"},{"line_number":529,"context_line":"            else:"},{"line_number":530,"context_line":"                self.log.debug(\"Creating new ProviderManager object for %s\" %"},{"line_number":531,"context_line":"                               p.name)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F31M%3D","line":528,"updated":"2013-08-20 21:57:58.000000000","message":"This reads a lot more clearly to me if instead of a new if block we made this an else to the if (p.username ...):","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"2b5da8c488d5a2d5e4c2123d03034272a65f02cf","unresolved":false,"context_lines":[{"line_number":525,"context_line":"                    stop_managers.append(oldmanager)"},{"line_number":526,"context_line":"                    oldmanager \u003d None"},{"line_number":527,"context_line":"            if oldmanager:"},{"line_number":528,"context_line":"                newconfig.managers[p.name] \u003d oldmanager"},{"line_number":529,"context_line":"            else:"},{"line_number":530,"context_line":"                self.log.debug(\"Creating new ProviderManager object for %s\" %"},{"line_number":531,"context_line":"                               p.name)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F3n8%3D","line":528,"in_reply_to":"AAAARn%2F%2F31M%3D","updated":"2013-08-20 22:25:39.000000000","message":"there are 2 ways we may not have an oldmanager: there never was one, or we have nulled it out because parameters changed.","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fc99e6de5d399007a94a5a03808409a3f0c2ce52","unresolved":false,"context_lines":[{"line_number":720,"context_line":"                server \u003d manager.getServer(node.external_id)"},{"line_number":721,"context_line":"                manager.cleanupServer(server[\u0027id\u0027])"},{"line_number":722,"context_line":"            except provider_manager.NotFound:"},{"line_number":723,"context_line":"                pass"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"        node.delete()"},{"line_number":726,"context_line":"        self.log.info(\"Deleted node id: %s\" % node.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F30g%3D","line":723,"updated":"2013-08-20 21:57:58.000000000","message":"Does this try except handle the two passes needed to delete a node?","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"2b5da8c488d5a2d5e4c2123d03034272a65f02cf","unresolved":false,"context_lines":[{"line_number":720,"context_line":"                server \u003d manager.getServer(node.external_id)"},{"line_number":721,"context_line":"                manager.cleanupServer(server[\u0027id\u0027])"},{"line_number":722,"context_line":"            except provider_manager.NotFound:"},{"line_number":723,"context_line":"                pass"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"        node.delete()"},{"line_number":726,"context_line":"        self.log.info(\"Deleted node id: %s\" % node.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F3nw%3D","line":723,"in_reply_to":"AAAARn%2F%2F30g%3D","updated":"2013-08-20 22:25:39.000000000","message":"The cleanupServer method waits for the server to be gone.  If it doesn\u0027t go away, it\u0027ll get picked up on the next pass.  That\u0027s not quite the same behavior as before, but it\u0027s close.  If we lower that timeout, it\u0027ll be closer.","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fc99e6de5d399007a94a5a03808409a3f0c2ce52","unresolved":false,"context_lines":[{"line_number":749,"context_line":"                                 snap_image.server_external_id)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        if server:"},{"line_number":752,"context_line":"            manager.cleanupServer(server[\u0027id\u0027])"},{"line_number":753,"context_line":""},{"line_number":754,"context_line":"        remote_image \u003d None"},{"line_number":755,"context_line":"        if snap_image.external_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F30E%3D","line":752,"updated":"2013-08-20 21:57:58.000000000","message":"For consistency this should probably got into the try on lines 745-746. This way it will match the deleteNode method.","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fc99e6de5d399007a94a5a03808409a3f0c2ce52","unresolved":false,"context_lines":[{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        if remote_image:"},{"line_number":763,"context_line":"            self.log.debug(\u0027Deleting image %s\u0027 % remote_image.id)"},{"line_number":764,"context_line":"            manager.deleteImage(remote_image[\u0027id\u0027])"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":"        snap_image.delete()"},{"line_number":767,"context_line":"        self.log.info(\"Deleted image id: %s\" % snap_image.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F3zY%3D","line":764,"updated":"2013-08-20 21:57:58.000000000","message":"Same comment here as the previous comment.","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"c2db6d0e55669269f0292188d254b4390998b8ca","unresolved":false,"context_lines":[{"line_number":521,"context_line":"                    p.auth_url !\u003d oldmanager.provider.auth_url or"},{"line_number":522,"context_line":"                    p.service_type !\u003d oldmanager.provider.service_type or"},{"line_number":523,"context_line":"                    p.service_name !\u003d oldmanager.provider.service_name or"},{"line_number":524,"context_line":"                    p.region_name !\u003d oldmanager.provider.region_name):"},{"line_number":525,"context_line":"                    stop_managers.append(oldmanager)"},{"line_number":526,"context_line":"                    oldmanager \u003d None"},{"line_number":527,"context_line":"            if oldmanager:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAARn%2F%2F3AI%3D","line":524,"updated":"2013-08-21 02:58:43.000000000","message":"I realise it\u0027s existing code but this sort of deep comparison really cries out for a simple dict you could compare:\nif p.details !\u003d oldmanager.provider.details:\n\n...","commit_id":"8dc6c870f26823c5163e8e613f6e76726b775fdd"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"c2db6d0e55669269f0292188d254b4390998b8ca","unresolved":false,"context_lines":[{"line_number":569,"context_line":"                    p.min_ready \u003d provider[\u0027min-ready\u0027]"},{"line_number":570,"context_line":"        self.config \u003d newconfig"},{"line_number":571,"context_line":"        for oldmanager in stop_managers:"},{"line_number":572,"context_line":"            oldmanager.stop()"},{"line_number":573,"context_line":"        if self.config.dburi !\u003d self.dburi:"},{"line_number":574,"context_line":"            self.dburi \u003d self.config.dburi"},{"line_number":575,"context_line":"            self.db \u003d nodedb.NodeDatabase(self.config.dburi)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAARn%2F%2F3AE%3D","line":572,"updated":"2013-08-21 02:58:43.000000000","message":"What happens if the manager throws an exception in stop? Do we end up wedged?","commit_id":"8dc6c870f26823c5163e8e613f6e76726b775fdd"}],"nodepool/provider_manager.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fc99e6de5d399007a94a5a03808409a3f0c2ce52","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        self.log.debug(\u0027Deleting server %s\u0027 % server_id)"},{"line_number":417,"context_line":"        self.deleteServer(server_id)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        for count in iterate_timeout(3600, \"waiting for server %s deletion\" %"},{"line_number":420,"context_line":"                                     server_id):"},{"line_number":421,"context_line":"            try:"},{"line_number":422,"context_line":"                self.getServer(server_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F3uo%3D","line":419,"updated":"2013-08-20 21:57:58.000000000","message":"Since we are serializing everything should this timeout be much shorter? Same question for the default timeout passed to _waitForResource above.","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"a65310fd5a1b6dfb59b277c75352470aceb9ca29","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        self.log.debug(\u0027Deleting server %s\u0027 % server_id)"},{"line_number":417,"context_line":"        self.deleteServer(server_id)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        for count in iterate_timeout(3600, \"waiting for server %s deletion\" %"},{"line_number":420,"context_line":"                                     server_id):"},{"line_number":421,"context_line":"            try:"},{"line_number":422,"context_line":"                self.getServer(server_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAARn%2F%2F3oU%3D","line":419,"in_reply_to":"AAAARn%2F%2F3uo%3D","updated":"2013-08-20 22:16:35.000000000","message":"This method and the _waitForResource calls above run in threads outside of the providerManager so will not prevent the manager threads from continueing to do their work. As a result these default timeout values are fine.","commit_id":"4f684212394b0c9815600354ea8f8a13e02afe48"}]}