)]}'
{"nova/compute/rpcapi.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e4778038c89a72410ae405535f17f0d4c6aa3b62","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                      {\u0027version\u0027: service_version})"},{"line_number":356,"context_line":"        return service_version, version_cap"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    def _determine_version_cap(self, target):"},{"line_number":359,"context_line":"        global LAST_VERSION"},{"line_number":360,"context_line":"        if LAST_VERSION:"},{"line_number":361,"context_line":"            return LAST_VERSION"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_5cd4021f","line":358,"updated":"2016-06-29 15:10:57.000000000","message":"Between the \"default client\" having a cap and different cell level caps this is getting a bit more complex to follow","commit_id":"fc93221c40ca468c8f9a6a121187dc5da6935913"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"713dc8c97142676c8183cfc9946171a07acababc","unresolved":false,"context_lines":[{"line_number":1068,"context_line":"                   delete_info\u003ddelete_info)"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"    def external_instance_event(self, ctxt, instances, events):"},{"line_number":1071,"context_line":"        instance \u003d instances[0]"},{"line_number":1072,"context_line":"        cctxt \u003d self.get_cell_client_instance(ctxt, instance).prepare("},{"line_number":1073,"context_line":"            server\u003d_compute_host(None, instance),"},{"line_number":1074,"context_line":"            version\u003d\u00274.0\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_76b9e8cb","line":1071,"updated":"2016-06-08 11:49:28.000000000","message":"Is it possible that several instances go to different cells?","commit_id":"fc93221c40ca468c8f9a6a121187dc5da6935913"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4c3ccfc26453ab59a3b8dbd819243c71e924bb5c","unresolved":false,"context_lines":[{"line_number":1068,"context_line":"                   delete_info\u003ddelete_info)"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"    def external_instance_event(self, ctxt, instances, events):"},{"line_number":1071,"context_line":"        instance \u003d instances[0]"},{"line_number":1072,"context_line":"        cctxt \u003d self.get_cell_client_instance(ctxt, instance).prepare("},{"line_number":1073,"context_line":"            server\u003d_compute_host(None, instance),"},{"line_number":1074,"context_line":"            version\u003d\u00274.0\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_2f2ebdfa","line":1071,"in_reply_to":"7aa08908_76b9e8cb","updated":"2016-06-08 21:21:17.000000000","message":"It seems not possible based on how this is called [1] one call per host is made passing a list of instances that are on that host. So it looks to me that this might be a \u0027get_cell_client_host\u0027 case actually.\n\n[1] https://github.com/openstack/nova/blob/3dd4b99/nova/compute/api.py#L3498-L3523","commit_id":"fc93221c40ca468c8f9a6a121187dc5da6935913"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4996b4ecc5493be5ecbf881eb2bbbe5db855c890","unresolved":false,"context_lines":[{"line_number":430,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":431,"context_line":"                                           block_migration, disk_over_commit):"},{"line_number":432,"context_line":"        version \u003d \u00274.11\u0027"},{"line_number":433,"context_line":"        client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":434,"context_line":"        if not client.can_send_version(version):"},{"line_number":435,"context_line":"            # NOTE(eliqiao): This is a new feature that is only available"},{"line_number":436,"context_line":"            # once all compute nodes support at least version 4.11."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_9d491bba","line":433,"updated":"2016-08-29 19:04:34.000000000","message":"I think this one should probably be \"by_host\" because it\u0027s definitely not the one that the instance is on right now. This probably isn\u0027t a problem until we can migrate between cells, but I think looking up by instance here is probably not right.\n\nWhat do you think?","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"dfa920007541da8c1617114f53529a9efdc4da8e","unresolved":false,"context_lines":[{"line_number":430,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":431,"context_line":"                                           block_migration, disk_over_commit):"},{"line_number":432,"context_line":"        version \u003d \u00274.11\u0027"},{"line_number":433,"context_line":"        client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":434,"context_line":"        if not client.can_send_version(version):"},{"line_number":435,"context_line":"            # NOTE(eliqiao): This is a new feature that is only available"},{"line_number":436,"context_line":"            # once all compute nodes support at least version 4.11."}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_c8bbdfe3","line":433,"in_reply_to":"fa7ab95a_9d491bba","updated":"2016-08-29 19:11:20.000000000","message":"Makes sense, and is more future-proof. We should probably document a guideline about this.","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4996b4ecc5493be5ecbf881eb2bbbe5db855c890","unresolved":false,"context_lines":[{"line_number":693,"context_line":"            host, migrate_data\u003dNone):"},{"line_number":694,"context_line":"        migrate_data_orig \u003d migrate_data"},{"line_number":695,"context_line":"        version \u003d \u00274.8\u0027"},{"line_number":696,"context_line":"        client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":697,"context_line":"        if not client.can_send_version(version):"},{"line_number":698,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":699,"context_line":"            if migrate_data:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_5d5dd345","line":696,"updated":"2016-08-29 19:04:34.000000000","message":"This routes to the host specified in the \"host\" parameter, so it should probably use by_host right?","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4996b4ecc5493be5ecbf881eb2bbbe5db855c890","unresolved":false,"context_lines":[{"line_number":726,"context_line":"                    \u0027node\u0027: node,"},{"line_number":727,"context_line":"                    \u0027clean_shutdown\u0027: clean_shutdown}"},{"line_number":728,"context_line":"        version \u003d \u00274.1\u0027"},{"line_number":729,"context_line":"        client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":730,"context_line":"        if not client.can_send_version(version):"},{"line_number":731,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":732,"context_line":"            msg_args[\u0027instance_type\u0027] \u003d objects_base.obj_to_primitive("}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_3d272fcb","line":729,"updated":"2016-08-29 19:04:34.000000000","message":"same here","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4996b4ecc5493be5ecbf881eb2bbbe5db855c890","unresolved":false,"context_lines":[{"line_number":798,"context_line":""},{"line_number":799,"context_line":"    def remove_volume_connection(self, ctxt, instance, volume_id, host):"},{"line_number":800,"context_line":"        version \u003d \u00274.0\u0027"},{"line_number":801,"context_line":"        cctxt \u003d self.router.by_instance(ctxt, instance).prepare("},{"line_number":802,"context_line":"                server\u003dhost, version\u003dversion)"},{"line_number":803,"context_line":"        return cctxt.call(ctxt, \u0027remove_volume_connection\u0027,"},{"line_number":804,"context_line":"                          instance\u003dinstance, volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_280c4b3a","line":801,"updated":"2016-08-29 19:04:34.000000000","message":"Also this","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4996b4ecc5493be5ecbf881eb2bbbe5db855c890","unresolved":false,"context_lines":[{"line_number":857,"context_line":"                                               destroy_disks\u003dTrue,"},{"line_number":858,"context_line":"                                               migrate_data\u003dNone):"},{"line_number":859,"context_line":"        version \u003d \u00274.8\u0027"},{"line_number":860,"context_line":"        client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":861,"context_line":"        if not client.can_send_version(version):"},{"line_number":862,"context_line":"            version \u003d \u00274.0\u0027"},{"line_number":863,"context_line":"            if migrate_data:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_8832b7f0","line":860,"updated":"2016-08-29 19:04:34.000000000","message":"This too.","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"b2934a95b4903f01b4c84bfe5476d3364eb45c91","unresolved":false,"context_lines":[{"line_number":491,"context_line":"    def confirm_resize(self, ctxt, instance, migration, host,"},{"line_number":492,"context_line":"            reservations\u003dNone, cast\u003dTrue):"},{"line_number":493,"context_line":"        version \u003d \u00274.0\u0027"},{"line_number":494,"context_line":"        if host is None:"},{"line_number":495,"context_line":"            client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":496,"context_line":"        else:"},{"line_number":497,"context_line":"            client \u003d self.router.by_host(ctxt, host)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_00d34260","line":494,"range":{"start_line":494,"start_character":8,"end_line":494,"end_character":23},"updated":"2016-08-31 19:30:06.000000000","message":"It\u0027s unfortunate that the host param doesn\u0027t default to None to indicate that this can happen.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"068e62bfc2d70b104469cbe8b883423638cf9b61","unresolved":false,"context_lines":[{"line_number":491,"context_line":"    def confirm_resize(self, ctxt, instance, migration, host,"},{"line_number":492,"context_line":"            reservations\u003dNone, cast\u003dTrue):"},{"line_number":493,"context_line":"        version \u003d \u00274.0\u0027"},{"line_number":494,"context_line":"        if host is None:"},{"line_number":495,"context_line":"            client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":496,"context_line":"        else:"},{"line_number":497,"context_line":"            client \u003d self.router.by_host(ctxt, host)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_b44bb858","line":494,"range":{"start_line":494,"start_character":8,"end_line":494,"end_character":23},"in_reply_to":"fa7ab95a_00d34260","updated":"2016-08-31 20:50:45.000000000","message":"Yeah, I assumed it could. However your comment makes me realize that in _compute_host it will do \u0027if host: return host\u0027 so comparing against None specifically won\u0027t give the same result here.\n\nI have kept thinking I should probably change these to call _compute_host ahead of time and use the returned value as the host, and only use by_host in this file. What do you think? If we do that, I wonder if I should just remove by_instance altogether.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"7b3fd7664434f73a9c6bbd47b37de0542756b34a","unresolved":false,"context_lines":[{"line_number":491,"context_line":"    def confirm_resize(self, ctxt, instance, migration, host,"},{"line_number":492,"context_line":"            reservations\u003dNone, cast\u003dTrue):"},{"line_number":493,"context_line":"        version \u003d \u00274.0\u0027"},{"line_number":494,"context_line":"        if host is None:"},{"line_number":495,"context_line":"            client \u003d self.router.by_instance(ctxt, instance)"},{"line_number":496,"context_line":"        else:"},{"line_number":497,"context_line":"            client \u003d self.router.by_host(ctxt, host)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_4fa815dc","line":494,"range":{"start_line":494,"start_character":8,"end_line":494,"end_character":23},"in_reply_to":"fa7ab95a_b44bb858","updated":"2016-08-31 21:10:22.000000000","message":"That would eliminate the chance of drift between what client.prepare is doing and what self.router is doing. I would be fine with that as a followup, but I like the idea.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"}],"nova/rpc.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"86b58070526072f69c7b069d91623b35fbc491f5","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":411,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"    def by_instance(self, context, instance):"},{"line_number":414,"context_line":"        try:"},{"line_number":415,"context_line":"            cell_mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":416,"context_line":"                               context, instance.uuid).cell_mapping"}],"source_content_type":"text/x-python","patch_set":5,"id":"3ac371cc_abd60a25","line":413,"updated":"2016-08-18 19:18:34.000000000","message":"This by_instance, by_host naming is very pleasing.","commit_id":"02f4df56b986ac9eb416477cc8ce8da3586278dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"731725e4920bb5cc652b74e2272d7776e1421e58","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        except KeyError:"},{"line_number":394,"context_line":"            # FIXME(melwitt): For temporary debugging"},{"line_number":395,"context_line":"            LOG.debug(\u0027Creating cell client with transport_url: %s\u0027,"},{"line_number":396,"context_line":"                      cell_mapping.transport_url)"},{"line_number":397,"context_line":"            transport \u003d create_transport(cell_mapping.transport_url)"},{"line_number":398,"context_line":"            client \u003d messaging.RPCClient(transport, self.target,"},{"line_number":399,"context_line":"                                         version_cap\u003dself.version_cap,"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_099af46d","line":396,"updated":"2016-08-30 14:09:27.000000000","message":"Meant to remove this?","commit_id":"5121af0b96817ef6e58d71d5a9993bca751f09f5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bd87d2a70e21331fe2e485a1cb0f9443ce17751d","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        except KeyError:"},{"line_number":394,"context_line":"            # FIXME(melwitt): For temporary debugging"},{"line_number":395,"context_line":"            LOG.debug(\u0027Creating cell client with transport_url: %s\u0027,"},{"line_number":396,"context_line":"                      cell_mapping.transport_url)"},{"line_number":397,"context_line":"            transport \u003d create_transport(cell_mapping.transport_url)"},{"line_number":398,"context_line":"            client \u003d messaging.RPCClient(transport, self.target,"},{"line_number":399,"context_line":"                                         version_cap\u003dself.version_cap,"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_895c414c","line":396,"in_reply_to":"fa7ab95a_099af46d","updated":"2016-08-30 14:48:07.000000000","message":"Yes, I put this to debug the grenade failures on this patch. It turns out the transport_url the cell mapping has in the db is missing the login userid for rabbit [1].\n\n[1] http://logs.openstack.org/06/326906/8/check/gate-grenade-dsvm-ubuntu-trusty/c4c7ec4/logs/new/screen-n-api.txt.gz#_2016-08-30_08_18_46_386","commit_id":"5121af0b96817ef6e58d71d5a9993bca751f09f5"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"b2934a95b4903f01b4c84bfe5476d3364eb45c91","unresolved":false,"context_lines":[{"line_number":380,"context_line":"        # NOTE(melwitt): Cells v1 does its own serialization and won\u0027t"},{"line_number":381,"context_line":"        # have a serializer available on the client object."},{"line_number":382,"context_line":"        self.serializer \u003d getattr(default_client, \u0027serializer\u0027, None)"},{"line_number":383,"context_line":"        self.run_periodic_tasks(nova.context.get_admin_context())"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    def _client(self, context, cell_mapping\u003dNone):"},{"line_number":386,"context_line":"        if cell_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_60e7ae12","line":383,"range":{"start_line":383,"start_character":32,"end_line":383,"end_character":64},"updated":"2016-08-31 19:30:06.000000000","message":"Does this need to be an admin context?","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7878a09bd17450f6aa7d6c6f56abf5d28aaf3bf1","unresolved":false,"context_lines":[{"line_number":380,"context_line":"        # NOTE(melwitt): Cells v1 does its own serialization and won\u0027t"},{"line_number":381,"context_line":"        # have a serializer available on the client object."},{"line_number":382,"context_line":"        self.serializer \u003d getattr(default_client, \u0027serializer\u0027, None)"},{"line_number":383,"context_line":"        self.run_periodic_tasks(nova.context.get_admin_context())"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    def _client(self, context, cell_mapping\u003dNone):"},{"line_number":386,"context_line":"        if cell_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_0f4d2d99","line":383,"range":{"start_line":383,"start_character":32,"end_line":383,"end_character":64},"in_reply_to":"fa7ab95a_2ff3d10b","updated":"2016-08-31 21:23:50.000000000","message":"Yeah, makes sense","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"7b3fd7664434f73a9c6bbd47b37de0542756b34a","unresolved":false,"context_lines":[{"line_number":380,"context_line":"        # NOTE(melwitt): Cells v1 does its own serialization and won\u0027t"},{"line_number":381,"context_line":"        # have a serializer available on the client object."},{"line_number":382,"context_line":"        self.serializer \u003d getattr(default_client, \u0027serializer\u0027, None)"},{"line_number":383,"context_line":"        self.run_periodic_tasks(nova.context.get_admin_context())"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    def _client(self, context, cell_mapping\u003dNone):"},{"line_number":386,"context_line":"        if cell_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_2ff3d10b","line":383,"range":{"start_line":383,"start_character":32,"end_line":383,"end_character":64},"in_reply_to":"fa7ab95a_3426c868","updated":"2016-08-31 21:10:22.000000000","message":"Okay. I\u0027ve been on a rampage about people using admin_context when not necessary so I\u0027d like to not have it here if not necessary. It seems like it wouldn\u0027t be. About the only place I\u0027ve seen that it\u0027s needed is for instance lookups which require either an admin or a user context, not a blank one.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"068e62bfc2d70b104469cbe8b883423638cf9b61","unresolved":false,"context_lines":[{"line_number":380,"context_line":"        # NOTE(melwitt): Cells v1 does its own serialization and won\u0027t"},{"line_number":381,"context_line":"        # have a serializer available on the client object."},{"line_number":382,"context_line":"        self.serializer \u003d getattr(default_client, \u0027serializer\u0027, None)"},{"line_number":383,"context_line":"        self.run_periodic_tasks(nova.context.get_admin_context())"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    def _client(self, context, cell_mapping\u003dNone):"},{"line_number":386,"context_line":"        if cell_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_3426c868","line":383,"range":{"start_line":383,"start_character":32,"end_line":383,"end_character":64},"in_reply_to":"fa7ab95a_60e7ae12","updated":"2016-08-31 20:50:45.000000000","message":"I\u0027m actually not sure. I copied what the other periodic tasks are doing. I think in my case it can be an empty context because I\u0027m not even using the context in my task.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"b2934a95b4903f01b4c84bfe5476d3364eb45c91","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        # Never expire the default client"},{"line_number":414,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":415,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def by_instance(self, context, instance):"},{"line_number":418,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_c0a87a7a","line":415,"updated":"2016-08-31 19:30:06.000000000","message":"Is the dict comprehension atomic from a python POV? I\u0027m just wondering if it will potentially conflict with lines 392 or 398 above?","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"068e62bfc2d70b104469cbe8b883423638cf9b61","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        # Never expire the default client"},{"line_number":414,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":415,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def by_instance(self, context, instance):"},{"line_number":418,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_74d130fb","line":415,"in_reply_to":"fa7ab95a_435ab42b","updated":"2016-08-31 20:50:45.000000000","message":"Hmm, good point. I doubt it\u0027s atomic. Let me look into this more.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7878a09bd17450f6aa7d6c6f56abf5d28aaf3bf1","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        # Never expire the default client"},{"line_number":414,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":415,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def by_instance(self, context, instance):"},{"line_number":418,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_af6c419d","line":415,"in_reply_to":"fa7ab95a_74d130fb","updated":"2016-08-31 21:23:50.000000000","message":"I think one problem with the way this is right now, even if comprehension is atomic, is clients could have been added on L398 after/during filtering out of stale clients, causing the client that was added to be excluded as though it was never added.\n\nI\u0027m wondering if maybe I should instead take a list copy of the current keys i.e. list(clients.keys()) and loop over that checking each one and removing it if it\u0027s stale. I think add/delete operations are atomic.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"7b3fd7664434f73a9c6bbd47b37de0542756b34a","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        # Never expire the default client"},{"line_number":414,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":415,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def by_instance(self, context, instance):"},{"line_number":418,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_cfe2652f","line":415,"in_reply_to":"fa7ab95a_74d130fb","updated":"2016-08-31 21:10:22.000000000","message":"May just be safer to create the dict and then assign it to self.clients in two steps and sidestep the concern. But now that I think more on it even if the dict creation isn\u0027t atomic the assignment to self.clients should be. So I think this is fine.\n\nExcept... this iterates through self.clients.items() which looks like it\u0027s safe to use even while the dict is being modified on line 398 above. But you may lose a new cached client while updating this here. If line 398 executes after self.client.items() is called and before self.clients is updated here. Not a big deal, but maybe a later thing to look at.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"aee9a32fc292ef382d47c1c5392175040dcf9ae0","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        # Never expire the default client"},{"line_number":414,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":415,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def by_instance(self, context, instance):"},{"line_number":418,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_a13a9532","line":415,"in_reply_to":"fa7ab95a_af6c419d","updated":"2016-08-31 21:39:03.000000000","message":"Yep. That sounds good.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5be2a4a41a9aaa2dd9befdafb0aaa1ccc0cc80fa","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        # Never expire the default client"},{"line_number":414,"context_line":"        self.clients \u003d {k: self.clients[k] for k, v in self.clients.items()"},{"line_number":415,"context_line":"                        if k \u003d\u003d \u0027default\u0027 or not stale(k, v.last_access_time)}"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def by_instance(self, context, instance):"},{"line_number":418,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_435ab42b","line":415,"in_reply_to":"fa7ab95a_c0a87a7a","updated":"2016-08-31 19:35:13.000000000","message":"I mean I assume the GIL won\u0027t leave this dict half finished, but just wondering if you knew.","commit_id":"b65dda4cd854ace9c14f8d5ed9130879fff1bdae"}]}