)]}'
{"nova/objects/compute_node.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8e275c7ee5f37f98b01d86e982f3c6f0dc3b02ed","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    fields \u003d {"},{"line_number":45,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":46,"context_line":"        \u0027service_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":47,"context_line":"        \u0027host\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":48,"context_line":"        \u0027vcpus\u0027: fields.IntegerField(),"},{"line_number":49,"context_line":"        \u0027memory_mb\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":4,"id":"dac4157f_a8e1c300","line":46,"updated":"2015-04-19 07:52:26.000000000","message":"Can we please add a unit test to check this if possible?","commit_id":"f938e6f9ecc1145066f4920f825666d5d51f3d31"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"a1ddb82eca7962d6d951f4bf7aa59f27bcc3ae54","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    fields \u003d {"},{"line_number":45,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":46,"context_line":"        \u0027service_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":47,"context_line":"        \u0027host\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":48,"context_line":"        \u0027vcpus\u0027: fields.IntegerField(),"},{"line_number":49,"context_line":"        \u0027memory_mb\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":4,"id":"dac4157f_472b2cec","line":46,"in_reply_to":"dac4157f_a8e1c300","updated":"2015-04-20 07:50:02.000000000","message":"Thank you for pointing this out. It think I will go even further and change the fake used for all tests to have a null value for service_id since this is what it will contain from here on.","commit_id":"f938e6f9ecc1145066f4920f825666d5d51f3d31"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57aac7363a41da358fe6e17b12f41122c1fb2298","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    fields \u003d {"},{"line_number":45,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":46,"context_line":"        \u0027service_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":47,"context_line":"        \u0027host\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":48,"context_line":"        \u0027vcpus\u0027: fields.IntegerField(),"},{"line_number":49,"context_line":"        \u0027memory_mb\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_22431344","line":46,"updated":"2015-05-28 22:45:58.000000000","message":"Now that this is nullable, you can\u0027t safely send this to a node that doesn\u0027t have nullable\u003dTrue on this attribute. If you can synthesize the value, or if it isn\u0027t actually used, then you can fix it in obj_make_compatible(). However, just making this change with no other compensation will break compatibility with older nodes.","commit_id":"7bc3946ba6ede2c27c653cf0bd5fa4f1cc3abb94"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a7169e55ab1c17aeae9266a58a3f2bc941625774","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    fields \u003d {"},{"line_number":45,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":46,"context_line":"        \u0027service_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":47,"context_line":"        \u0027host\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":48,"context_line":"        \u0027vcpus\u0027: fields.IntegerField(),"},{"line_number":49,"context_line":"        \u0027memory_mb\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_bfd95259","line":46,"in_reply_to":"7a016987_22431344","updated":"2015-05-29 08:20:56.000000000","message":"Oh right, we need a backwards compatibility with older computes. Something like :\n\n if target_version \u003c (1, 12) and (primitive[\u0027service_id\u0027] is None or service_id not in primitive):\n  primitive[service_id] \u003d objects.Service.get_by_host_and_nodename(...)","commit_id":"7bc3946ba6ede2c27c653cf0bd5fa4f1cc3abb94"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4011985d8a016577d63bc7de1ded94815659b16d","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    fields \u003d {"},{"line_number":45,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":46,"context_line":"        \u0027service_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":47,"context_line":"        \u0027host\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":48,"context_line":"        \u0027vcpus\u0027: fields.IntegerField(),"},{"line_number":49,"context_line":"        \u0027memory_mb\u0027: fields.IntegerField(),"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_9fa8d68d","line":46,"in_reply_to":"7a016987_bfd95259","updated":"2015-05-29 08:23:16.000000000","message":"Of course, please invert the \u0027or\u0027...","commit_id":"7bc3946ba6ede2c27c653cf0bd5fa4f1cc3abb94"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"561582109b4e40bb50d03d0e929531beafa0836c","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":84,"context_line":"        if target_version \u003c (1, 12) and primitive.get(\u0027service_id\u0027) is None:"},{"line_number":85,"context_line":"            # service_id is non-nullable in versions before 1.12"},{"line_number":86,"context_line":"            try:"},{"line_number":87,"context_line":"                service \u003d objects.Service.get_by_compute_host("},{"line_number":88,"context_line":"                    self._context, primitive[\u0027host\u0027])"},{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_6390dced","line":86,"updated":"2015-05-29 12:16:37.000000000","message":"The exception handling here is mostly due to how this gets exercised in test_objects without data. Without it this blows up with a KeyError on primitive[\u0027host\u0027].","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2e1f11f85445884bc45adede56c8cebe08ed6a17","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":84,"context_line":"        if target_version \u003c (1, 12) and primitive.get(\u0027service_id\u0027) is None:"},{"line_number":85,"context_line":"            # service_id is non-nullable in versions before 1.12"},{"line_number":86,"context_line":"            try:"},{"line_number":87,"context_line":"                service \u003d objects.Service.get_by_compute_host("},{"line_number":88,"context_line":"                    self._context, primitive[\u0027host\u0027])"},{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_eed95357","line":86,"in_reply_to":"7a016987_6390dced","updated":"2015-05-29 12:26:37.000000000","message":"Then you should instead except only (ComputeHostNotFound, KeyError)","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"923af522252d6a180102cb5de204984e3477b910","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":84,"context_line":"        if target_version \u003c (1, 12) and primitive.get(\u0027service_id\u0027) is None:"},{"line_number":85,"context_line":"            # service_id is non-nullable in versions before 1.12"},{"line_number":86,"context_line":"            try:"},{"line_number":87,"context_line":"                service \u003d objects.Service.get_by_compute_host("},{"line_number":88,"context_line":"                    self._context, primitive[\u0027host\u0027])"},{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_299645f0","line":86,"in_reply_to":"7a016987_eed95357","updated":"2015-05-29 13:25:11.000000000","message":"Done","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2e1f11f85445884bc45adede56c8cebe08ed6a17","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                service \u003d objects.Service.get_by_compute_host("},{"line_number":88,"context_line":"                    self._context, primitive[\u0027host\u0027])"},{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except Exception:"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_4eb86711","line":90,"updated":"2015-05-29 12:26:37.000000000","message":"see above, I prefer to except what we know","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2e1f11f85445884bc45adede56c8cebe08ed6a17","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except Exception:"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":95,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_ae735b7f","line":92,"updated":"2015-05-29 12:26:37.000000000","message":"Mmm, just wondering if it would be worth leaving the ComputeHostNotFound exception being raised because if we can\u0027t find the related Service, then the compute node is wrong IMHO.","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"561582109b4e40bb50d03d0e929531beafa0836c","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except Exception:"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":95,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_835cf84b","line":92,"updated":"2015-05-29 12:16:37.000000000","message":"Not entirely sure about this. However, service_id is not used anywhere that I know of in kilo, and that is what matters I guess.","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b6d28f647fdfb1a3c5224a43b0aabac13a6df068","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except Exception:"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":95,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_14660051","line":92,"in_reply_to":"7a016987_140ba0d7","updated":"2015-05-29 13:27:40.000000000","message":"Fair point","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"923af522252d6a180102cb5de204984e3477b910","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except Exception:"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":95,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_140ba0d7","line":92,"in_reply_to":"7a016987_ae735b7f","updated":"2015-05-29 13:25:11.000000000","message":"I personally think we should stop making that assumption. We want to decouple service and compute_node, not make the relation tighter. Also, we don\u0027t know if callers (current or future ones) care for the service status when they lookup this table. If they do, they should themselves call is_up() or something on the service, which I think most do.","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"561582109b4e40bb50d03d0e929531beafa0836c","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    @property"},{"line_number":277,"context_line":"    def service(self):"},{"line_number":278,"context_line":"        if not hasattr(self, \u0027_cached_service\u0027):"},{"line_number":279,"context_line":"            self._cached_service \u003d objects.Service.get_by_id(self._context,"},{"line_number":280,"context_line":"                                                             self.service_id)"},{"line_number":281,"context_line":"        return self._cached_service"},{"line_number":282,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a016987_4e2e278d","line":279,"updated":"2015-05-29 12:16:37.000000000","message":"...well actually service_id is referenced here, but this property is unused too. I have a patch in the works to remove it that will be up shortly.","commit_id":"1930c9b0dac06957f6497f58d3d15f7612edee49"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"424b69f2a454510060b9da286ad51a0c37090541","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                    self._context, primitive[\u0027host\u0027])"},{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except (exception.ComputeHostNotFound, KeyError):"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_544358af","line":91,"updated":"2015-05-29 13:26:52.000000000","message":"Well, the comment is not so terrible since it doesn\u0027t explain why you add a KeyError, but that\u0027s fair, we can live with it.\n\nHave you tried to modify the tests to add the host field to the existing ComputeNode objects when it was raising a KeyError to see if we can remove that exception case ?","commit_id":"399302bdc3d4c2d96b3b0ae0ceef7958f71f67c8"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"a08b1118da6afea4fc4b37846b4a09fb5f2f818f","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                    self._context, primitive[\u0027host\u0027])"},{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except (exception.ComputeHostNotFound, KeyError):"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_affa35bb","line":91,"in_reply_to":"7a016987_544358af","updated":"2015-05-29 13:46:02.000000000","message":"Yeah, looked at it but didn\u0027t want to make special handling for ComputeNode in those tests. For example test_obj_make_compatible here: https://github.com/openstack/nova/blob/master/nova/tests/unit/objects/test_objects.py#L1334","commit_id":"399302bdc3d4c2d96b3b0ae0ceef7958f71f67c8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"424b69f2a454510060b9da286ad51a0c37090541","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except (exception.ComputeHostNotFound, KeyError):"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":95,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_741d34bd","line":92,"updated":"2015-05-29 13:26:52.000000000","message":"You should also maybe explain that setting this field to -1 will just leave a ServiceNotFound so that\u0027s fine","commit_id":"399302bdc3d4c2d96b3b0ae0ceef7958f71f67c8"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"45ee5a85129c3d0fc31ad83963d36c4e7c278310","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                primitive[\u0027service_id\u0027] \u003d service.id"},{"line_number":90,"context_line":"            except (exception.ComputeHostNotFound, KeyError):"},{"line_number":91,"context_line":"                # Just in case..."},{"line_number":92,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":93,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":94,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":95,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_fe9d6966","line":92,"in_reply_to":"7a016987_741d34bd","updated":"2015-05-30 07:47:42.000000000","message":"Since I had to do another spin I updated this with some more info.","commit_id":"399302bdc3d4c2d96b3b0ae0ceef7958f71f67c8"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"afe508e8e55d8b68529b234554218da9eb6b94c2","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                # to allow for older versions that demand a value to work."},{"line_number":97,"context_line":"                # Setting to -1 will, if value is later used result in a"},{"line_number":98,"context_line":"                # ServiceNotFound, so should be safe."},{"line_number":99,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":100,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":101,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":102,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a4dcd0f_60fffa34","line":99,"updated":"2015-08-10 10:08:39.000000000","message":"So it seems that there is no scenario with the way we use SQLA where there would be some confusion with an accidental unsigned data type - so this should be fine.\n\nsomewhat related: https://stackoverflow.com/questions/19717738/how-to-define-an-unsigned-integer-in-sqlalchemy","commit_id":"b2535136d2fe300dcf5dc843e919c6c26d09edac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"82d337b51ce0813f39fdee790a492f31b7f434c0","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                # to allow for older versions that demand a value to work."},{"line_number":97,"context_line":"                # Setting to -1 will, if value is later used result in a"},{"line_number":98,"context_line":"                # ServiceNotFound, so should be safe."},{"line_number":99,"context_line":"                primitive[\u0027service_id\u0027] \u003d -1"},{"line_number":100,"context_line":"        if target_version \u003c (1, 7) and \u0027host\u0027 in primitive:"},{"line_number":101,"context_line":"            del primitive[\u0027host\u0027]"},{"line_number":102,"context_line":"        if target_version \u003c (1, 5) and \u0027numa_topology\u0027 in primitive:"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa1b9901_714c579e","line":99,"updated":"2015-08-19 17:25:53.000000000","message":"Excellent.","commit_id":"0b177f18264ca842e16c2c989a081e773b3d86b7"}],"nova/objects/service.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e30e43b4c234faabec95038bbccfedf7647f0fb3","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            return"},{"line_number":86,"context_line":"        # NOTE(hanlind): Make sure service_id is set to save us from having to"},{"line_number":87,"context_line":"        # make another db lookup just to get the service id."},{"line_number":88,"context_line":"        compute.service_id \u003d primitive[\u0027id\u0027]"},{"line_number":89,"context_line":"        primitive[\u0027compute_node\u0027] \u003d compute.obj_to_primitive("},{"line_number":90,"context_line":"            target_version\u003dtarget_version)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_8f33cf13","line":88,"updated":"2015-06-01 08:29:55.000000000","message":"While I understand your choice to prevent a DB lookup if the service_id is not there, I think we should not add this just for removing a DB call, since it will keep the relationship explicitely, while we want actually to remove it mid-term.\n\nThere are 3 possibilities here: \n - Controller is Liberty and computes are Liberty : When calling obj_to_primitive(), it won\u0027t go into the backwards compat code where it calls the Service to get the service_id.\nIn that case, adding compute.service_id just goes in the opposite direction that you want to provide\n\n - Controller is Kilo and computes are Kilo : service_id is still provided by the RT (and consequently got when calling the object method above), there is no reason to add the value if it\u0027s still present\n\n - Controller is Liberty (well, post-change) and Computes are Kilo : in that case, the compat code will be executed and you will make sure that service_id is populated. Even it that\u0027s expensive, that\u0027s the only case where it\u0027s necessary.","commit_id":"7be2c4c4754afbc06ecf8400aab8797a374fab03"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1460cecd3add910daef6579a87fda5f3ed6c9edf","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            return"},{"line_number":86,"context_line":"        # NOTE(hanlind): Make sure service_id is set to save us from having to"},{"line_number":87,"context_line":"        # make another db lookup just to get the service id."},{"line_number":88,"context_line":"        compute.service_id \u003d primitive[\u0027id\u0027]"},{"line_number":89,"context_line":"        primitive[\u0027compute_node\u0027] \u003d compute.obj_to_primitive("},{"line_number":90,"context_line":"            target_version\u003dtarget_version)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_d87f44a9","line":88,"in_reply_to":"7a016987_00756c62","updated":"2015-06-01 15:04:17.000000000","message":"Oh right, that\u0027s only an helper method for pre-lazy loading compute nodes which means that service_id is always seen as needed. \n\nThat said, correct me if I\u0027m wrong, as this codepath is only called for pre-Liberty computes, we\u0027re 100% sure that those will have the service_id field set as the RT will continue to provide it until a commit removes https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L339\n\nOnce this line is removed, new Liberty computes won\u0027t provide service_id but this compat codepath will never be executed anyway.\n\ntl;dr: by now, all computes (even master ones) are still continuing to provide service_id, there is no need to add this.","commit_id":"7be2c4c4754afbc06ecf8400aab8797a374fab03"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"92c70f69823e9a86b715a25d1e1353dccb793b14","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            return"},{"line_number":86,"context_line":"        # NOTE(hanlind): Make sure service_id is set to save us from having to"},{"line_number":87,"context_line":"        # make another db lookup just to get the service id."},{"line_number":88,"context_line":"        compute.service_id \u003d primitive[\u0027id\u0027]"},{"line_number":89,"context_line":"        primitive[\u0027compute_node\u0027] \u003d compute.obj_to_primitive("},{"line_number":90,"context_line":"            target_version\u003dtarget_version)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_00756c62","line":88,"in_reply_to":"7a016987_8f33cf13","updated":"2015-06-01 09:52:23.000000000","message":"I think you are missing that this method (_do_compute_node) is only ever called for back ports to versions \u003c1.10.\n\nNow, we make sure back ports all the way to 1.0 continues to work, this is no different, but in reality the only way for this to be called would be if the controller was Liberty (including this change) and computes were Juno or older.","commit_id":"7be2c4c4754afbc06ecf8400aab8797a374fab03"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"6eb717871b60ddefa81c4195fdd003aab5ffb494","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            return"},{"line_number":86,"context_line":"        # NOTE(hanlind): Make sure service_id is set to save us from having to"},{"line_number":87,"context_line":"        # make another db lookup just to get the service id."},{"line_number":88,"context_line":"        compute.service_id \u003d primitive[\u0027id\u0027]"},{"line_number":89,"context_line":"        primitive[\u0027compute_node\u0027] \u003d compute.obj_to_primitive("},{"line_number":90,"context_line":"            target_version\u003dtarget_version)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_9f9d0b5c","line":88,"in_reply_to":"7a016987_d87f44a9","updated":"2015-06-02 09:41:44.000000000","message":"It\u0027s actually a bit more complex (or at least it could have been). Ex. an older compute node queries for a Liberty compute node record during live-migration, assuming the Liberty one no longer reports service_id. This is a scenario that could have been relevant, however this would only be for a pre-Kilo compute against this code in Liberty so in reality this code path is never ever going to be executed.\n\nOne could also say that keeping compat code for cn \u003c 1.10 in Liberty is kind of waste.\n\nI\u0027ll remove this in a new ps since clearly it will never be executed.","commit_id":"7be2c4c4754afbc06ecf8400aab8797a374fab03"}],"nova/tests/unit/objects/test_service.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"07e04c4ce8b3f164e6d0b056921df80a61dd8a8d","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        service_obj \u003d objects.Service(context\u003dself.context)"},{"line_number":240,"context_line":"        fake_service_dict \u003d fake_service.copy()"},{"line_number":241,"context_line":"        fake_compute_obj \u003d objects.ComputeNode(host\u003dfake_service[\u0027host\u0027],"},{"line_number":242,"context_line":"                                               service_id\u003dfake_service[\u0027id\u0027])"},{"line_number":243,"context_line":"        get_all_by_host.return_value \u003d [fake_compute_obj]"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        service_obj.obj_make_compatible(fake_service_dict, \u00271.9\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1af86dd1_4360c735","line":242,"updated":"2015-06-12 15:13:35.000000000","message":"I don\u0027t really like adding service_id as a parameter here, since we really want to remove all of it, but here that\u0027s fine as the unittest is checking by old compute node.","commit_id":"116aa07a25b21c5019895dc0d98e06af7dda2f6b"}]}