)]}'
{"nova/db/api.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"44c12a9abfa8abcae1a5dec4cea1e1fc3a4f0109","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    return IMPL.compute_node_get(context, compute_id)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"def compute_node_get_by_service_id(context, service_id, use_slave\u003dFalse):"},{"line_number":171,"context_line":"    \"\"\"Get a compute node by its associated service id."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    :param context: The security context"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_29dc7a18","line":170,"updated":"2014-12-15 22:00:13.000000000","message":"Is there a reason you added the use_slave kwarg in this patch?","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06770514644c45b29a0d4109adbf74b98fd4a584","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    return IMPL.compute_node_get(context, compute_id)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"def compute_node_get_by_service_id(context, service_id, use_slave\u003dFalse):"},{"line_number":171,"context_line":"    \"\"\"Get a compute node by its associated service id."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    :param context: The security context"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_a52b1093","line":170,"in_reply_to":"3a961159_0c8c2c7f","updated":"2014-12-16 17:20:57.000000000","message":"Well, as now the method will return a list of objects, I was thinking it was good to add a slave argument as for the other list methods... But sure, I\u0027ll remove the arg.","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bf8b038f68dc4d319d38ce624c93e1c541f5c090","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    return IMPL.compute_node_get(context, compute_id)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"def compute_node_get_by_service_id(context, service_id, use_slave\u003dFalse):"},{"line_number":171,"context_line":"    \"\"\"Get a compute node by its associated service id."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    :param context: The security context"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_0c8c2c7f","line":170,"in_reply_to":"3a961159_29dc7a18","updated":"2014-12-15 22:30:42.000000000","message":"Agreed, this should be a separate patch, if it\u0027s needed for some reason. Sounds like we\u0027ll be getting some slave lovin\u0027 for free with the enginefacade stuff, so I would tend to think we should stop adding these flags at this point.","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bf8b038f68dc4d319d38ce624c93e1c541f5c090","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    :param service_id: ID of the associated service"},{"line_number":175,"context_line":"    :param use_slave: Use slave DB"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    :returns: List of dictionaries containing properties of the compute node,"},{"line_number":178,"context_line":"              including its corresponding service and statistics"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    Raises ServiceNotFound if service with the given ID doesn\u0027t exist."}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_cc91449c","line":177,"updated":"2014-12-15 22:30:42.000000000","message":"Is this really correct? It should be something like \"list of dictionary-like objects, each representing a compute node (and its properties) associated with the service.\"","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06770514644c45b29a0d4109adbf74b98fd4a584","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    :param service_id: ID of the associated service"},{"line_number":175,"context_line":"    :param use_slave: Use slave DB"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    :returns: List of dictionaries containing properties of the compute node,"},{"line_number":178,"context_line":"              including its corresponding service and statistics"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    Raises ServiceNotFound if service with the given ID doesn\u0027t exist."}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_e51c387a","line":177,"in_reply_to":"3a961159_cc91449c","updated":"2014-12-16 17:20:57.000000000","message":"Done","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"043e40db277e3f869424b85cdb94594119f279b9","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"def compute_nodes_get_by_service_id(context, service_id):"},{"line_number":171,"context_line":"    \"\"\"Get a compute node by its associated service id."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    :param context: The security context"},{"line_number":174,"context_line":"    :param service_id: ID of the associated service"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_f25c674e","line":171,"updated":"2015-01-15 19:44:37.000000000","message":"Forgot to update this to plural compute nodes like the returns.","commit_id":"027e7b64af7009f481c6835342b9ba451389bee0"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b758b4d2ec209097280ad0d456beb5351f319783","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"@require_admin_context"},{"line_number":497,"context_line":"def compute_node_get_by_service_id(context, service_id):"},{"line_number":498,"context_line":"    result \u003d model_query(context, models.ComputeNode, read_deleted\u003d\u0027no\u0027).\\"},{"line_number":499,"context_line":"        filter_by(service_id\u003dservice_id).\\"},{"line_number":500,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_545999fb","line":497,"updated":"2015-01-07 20:22:35.000000000","message":"Shouldn\u0027t we change the name of this to reflect that it returns more than one thing? There are other places where we didn\u0027t pluralize compute_node and it\u0027s been no end of confusion. I think compute_nodes_get_by_service_id() would be much better.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144db5da87503264353de0e9356cad3fbe39dfac","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"@require_admin_context"},{"line_number":497,"context_line":"def compute_node_get_by_service_id(context, service_id):"},{"line_number":498,"context_line":"    result \u003d model_query(context, models.ComputeNode, read_deleted\u003d\u0027no\u0027).\\"},{"line_number":499,"context_line":"        filter_by(service_id\u003dservice_id).\\"},{"line_number":500,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_42d40dbd","line":497,"in_reply_to":"3a961159_2c5d687d","updated":"2015-01-12 21:27:29.000000000","message":"Done","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e9bb6f30f42b4c63ade0993f5de63b68f7cbda4","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"@require_admin_context"},{"line_number":497,"context_line":"def compute_node_get_by_service_id(context, service_id):"},{"line_number":498,"context_line":"    result \u003d model_query(context, models.ComputeNode, read_deleted\u003d\u0027no\u0027).\\"},{"line_number":499,"context_line":"        filter_by(service_id\u003dservice_id).\\"},{"line_number":500,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_0f845e5c","line":497,"in_reply_to":"3a961159_545999fb","updated":"2015-01-07 20:38:14.000000000","message":"+1","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"33e425beee4d83646a296d46d9a9b61653cc212a","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"@require_admin_context"},{"line_number":497,"context_line":"def compute_node_get_by_service_id(context, service_id):"},{"line_number":498,"context_line":"    result \u003d model_query(context, models.ComputeNode, read_deleted\u003d\u0027no\u0027).\\"},{"line_number":499,"context_line":"        filter_by(service_id\u003dservice_id).\\"},{"line_number":500,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_e5fe5105","line":497,"in_reply_to":"3a961159_545999fb","updated":"2015-01-07 22:23:17.000000000","message":"We are deprecating this method in the next patch in the series. I really don\u0027t think it\u0027s worth the effort...","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4701802c4ae0a476ba112be2a8ca75c48c5787f1","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"@require_admin_context"},{"line_number":497,"context_line":"def compute_node_get_by_service_id(context, service_id):"},{"line_number":498,"context_line":"    result \u003d model_query(context, models.ComputeNode, read_deleted\u003d\u0027no\u0027).\\"},{"line_number":499,"context_line":"        filter_by(service_id\u003dservice_id).\\"},{"line_number":500,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_e9546bff","line":497,"in_reply_to":"3a961159_e5fe5105","updated":"2015-01-07 23:30:57.000000000","message":"We have to keep this around until we drop compatibility for the RPC versions that depend on it. That means it stays in the DB api until that point (see my comment on the later patch).\n\nIt\u0027s only used in a single place, right? This patch already touches this method in db api, and here.. making the name change will be one more line of code so it doesn\u0027t seem like a lot of work to me.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"becbd1669ac346d04ac392aa394e494d03d6bd0a","unresolved":false,"context_lines":[{"line_number":494,"context_line":""},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"@require_admin_context"},{"line_number":497,"context_line":"def compute_node_get_by_service_id(context, service_id):"},{"line_number":498,"context_line":"    result \u003d model_query(context, models.ComputeNode, read_deleted\u003d\u0027no\u0027).\\"},{"line_number":499,"context_line":"        filter_by(service_id\u003dservice_id).\\"},{"line_number":500,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_2c5d687d","line":497,"in_reply_to":"3a961159_e9546bff","updated":"2015-01-09 20:38:57.000000000","message":"I don\u0027t like the singular/plural inconsistency either, and if this was going to remain active code, I would insist on fixing it. But I have to agree with Jay that as it\u0027s going away, and it\u0027s handled in the few places it\u0027s called, that we should note it and move ahead.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"}],"nova/objects/compute_node.py":[{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"5a3a5e368c0750da644e066ac6ff91905fa54dae","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    @base.remotable_classmethod"},{"line_number":278,"context_line":"    def _get_by_service(cls, context, service_id, use_slave\u003dFalse):"},{"line_number":279,"context_line":"        db_computes \u003d db.compute_node_get_by_service_id(context, service_id,"},{"line_number":280,"context_line":"                                                        use_slave)"},{"line_number":281,"context_line":"        return base.obj_make_list(context, cls(context), objects.ComputeNode,"},{"line_number":282,"context_line":"                                  db_computes)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_4ad3915f","line":279,"updated":"2014-12-15 09:54:52.000000000","message":"I think you need a try/catch around this and return an empty list in case of ServiceNotFound to retain the same behavior as before.\n\nCurrently, if the service record exists but no compute nodes are, this will throw.","commit_id":"25006a2675b007f4121c9283004f207bda158fbf"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"19ebe8649cd3448d90a88d07a23647eadb4035e7","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    @base.remotable_classmethod"},{"line_number":278,"context_line":"    def _get_by_service(cls, context, service_id, use_slave\u003dFalse):"},{"line_number":279,"context_line":"        db_computes \u003d db.compute_node_get_by_service_id(context, service_id,"},{"line_number":280,"context_line":"                                                        use_slave)"},{"line_number":281,"context_line":"        return base.obj_make_list(context, cls(context), objects.ComputeNode,"},{"line_number":282,"context_line":"                                  db_computes)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_33487cb2","line":279,"in_reply_to":"3a961159_4ad3915f","updated":"2014-12-15 11:29:37.000000000","message":"You overbeated me by 30 mins... I was looking at the bug when I just updated this page and saw your comment.","commit_id":"25006a2675b007f4121c9283004f207bda158fbf"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"44c12a9abfa8abcae1a5dec4cea1e1fc3a4f0109","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # NOTE(sbauza): Old version was returning an item, we need to keep this"},{"line_number":148,"context_line":"        # behaviour for backwards compatibility"},{"line_number":149,"context_line":"        db_compute \u003d db_computes[0]"},{"line_number":150,"context_line":"        return cls._from_db_object(context, cls(), db_compute)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @base.remotable_classmethod"},{"line_number":153,"context_line":"    def get_by_host_and_nodename(cls, context, host, nodename):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_49e1c6cf","line":150,"updated":"2014-12-15 22:00:13.000000000","message":"Sylvain, I don\u0027t understand this... in the next patch in the series, you remove the callers of ComputeNode.get_by_service_id(). Why are you bothering to fix the DB API compute_node_get_by_service_id() method in this patch?","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06770514644c45b29a0d4109adbf74b98fd4a584","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # NOTE(sbauza): Old version was returning an item, we need to keep this"},{"line_number":148,"context_line":"        # behaviour for backwards compatibility"},{"line_number":149,"context_line":"        db_compute \u003d db_computes[0]"},{"line_number":150,"context_line":"        return cls._from_db_object(context, cls(), db_compute)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    @base.remotable_classmethod"},{"line_number":153,"context_line":"    def get_by_host_and_nodename(cls, context, host, nodename):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_b7518310","line":150,"in_reply_to":"3a961159_49e1c6cf","updated":"2014-12-16 17:20:57.000000000","message":"Yeah, it\u0027s necessary to fix this method because I\u0027m using it for compatibility purpose in the ComputeNode object. Even in the next patches, I\u0027ll leave this method but I\u0027ll deprecate it so that only the ComputeNode object will call it.\n\nThat\u0027s really necessary because the previous method was returning the first element when we were having 2 nodes for the same host so consequently my previous patch was buggy.","commit_id":"ec92e92fab9eb397c138174462786b831cfa6ef3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07682bd8f2bc15d2c7476557c6382afc4b42769a","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Version 1.6: Added supported_hv_specs"},{"line_number":36,"context_line":"    # Version 1.7: Added host field"},{"line_number":37,"context_line":"    # Version 1.8: Added get_by_host_and_nodename()"},{"line_number":38,"context_line":"    # Version 1.9: Added pci_device_pools"},{"line_number":39,"context_line":"    VERSION \u003d \u00271.9\u0027"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_31fc1f72","line":38,"updated":"2015-01-07 20:04:07.000000000","message":"I was wondering about a version bump but it *looks* like you\u0027ve done a good job of maintaining backwards compatibility.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a2aab54b858ac4986d4036ad743d0e403ed6d96a","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Version 1.6: Added supported_hv_specs"},{"line_number":36,"context_line":"    # Version 1.7: Added host field"},{"line_number":37,"context_line":"    # Version 1.8: Added get_by_host_and_nodename()"},{"line_number":38,"context_line":"    # Version 1.9: Added pci_device_pools"},{"line_number":39,"context_line":"    VERSION \u003d \u00271.9\u0027"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_7d4e0ac4","line":38,"in_reply_to":"3a961159_31fc1f72","updated":"2015-01-12 21:30:57.000000000","message":"Correct, I only provided some backwards compatibility with no new signatures,so it doesn\u0027t need a version bump","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b758b4d2ec209097280ad0d456beb5351f319783","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # call but that\u0027s necessary until all nodes are upgraded."},{"line_number":178,"context_line":"            try:"},{"line_number":179,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host)"},{"line_number":180,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":181,"context_line":"                    context, service.id)"},{"line_number":182,"context_line":"            except exception.ServiceNotFound:"},{"line_number":183,"context_line":"                # We need to provide the same exception upstream"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_140ab1d1","line":180,"updated":"2015-01-07 20:22:35.000000000","message":"See, this seems awkward to me. You\u0027re setting a plural variable from a singular query.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"33e425beee4d83646a296d46d9a9b61653cc212a","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # call but that\u0027s necessary until all nodes are upgraded."},{"line_number":178,"context_line":"            try:"},{"line_number":179,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host)"},{"line_number":180,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":181,"context_line":"                    context, service.id)"},{"line_number":182,"context_line":"            except exception.ServiceNotFound:"},{"line_number":183,"context_line":"                # We need to provide the same exception upstream"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_a00fd72c","line":180,"in_reply_to":"3a961159_140ab1d1","updated":"2015-01-07 22:23:17.000000000","message":"Yeah, it\u0027s awkward because of the original design of the relationship between compute node and service. There actually can never be more than 1 record returned for a compute node.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144db5da87503264353de0e9356cad3fbe39dfac","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # call but that\u0027s necessary until all nodes are upgraded."},{"line_number":178,"context_line":"            try:"},{"line_number":179,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host)"},{"line_number":180,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":181,"context_line":"                    context, service.id)"},{"line_number":182,"context_line":"            except exception.ServiceNotFound:"},{"line_number":183,"context_line":"                # We need to provide the same exception upstream"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_9d31f6ef","line":180,"in_reply_to":"3a961159_a00fd72c","updated":"2015-01-12 21:27:29.000000000","message":"Done","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07682bd8f2bc15d2c7476557c6382afc4b42769a","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":185,"context_line":"            db_compute \u003d None"},{"line_number":186,"context_line":"            for compute in db_computes:"},{"line_number":187,"context_line":"                if compute[\u0027hypervisor_hostname\u0027] \u003d\u003d nodename:"},{"line_number":188,"context_line":"                    db_compute \u003d compute"},{"line_number":189,"context_line":"                    # We can avoid an extra call to Service object in"},{"line_number":190,"context_line":"                    # _from_db_object"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_d1411ba7","line":187,"updated":"2015-01-07 20:04:07.000000000","message":"Do we have a test somewhere where there are multiple compute nodes for a given service so we have to filter by the hostname (not the NotFound case)?  I see there is a test where we get NotFound because the hostname is danplanet but the nodename passed in is \u0027elsewhere\u0027 or something, and we have tests where there is a single compute node for the service and the hostname matches what\u0027s passed in, but what about multiple compute nodes where the first hostname doesn\u0027t match but the second does?  That would test that it doesn\u0027t just return the first one found.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f564409124c502f16597f1c8c96939f72bf61e7b","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":185,"context_line":"            db_compute \u003d None"},{"line_number":186,"context_line":"            for compute in db_computes:"},{"line_number":187,"context_line":"                if compute[\u0027hypervisor_hostname\u0027] \u003d\u003d nodename:"},{"line_number":188,"context_line":"                    db_compute \u003d compute"},{"line_number":189,"context_line":"                    # We can avoid an extra call to Service object in"},{"line_number":190,"context_line":"                    # _from_db_object"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_e9b1cb01","line":187,"in_reply_to":"3a961159_406c73d0","updated":"2015-01-07 23:41:02.000000000","message":"You could for the ironic case, right? This would have just return the first compute node, which may or may not have been right (I dunno). Also, vmware uses multiple compute nodes under a single compute service, right?","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"33e425beee4d83646a296d46d9a9b61653cc212a","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":185,"context_line":"            db_compute \u003d None"},{"line_number":186,"context_line":"            for compute in db_computes:"},{"line_number":187,"context_line":"                if compute[\u0027hypervisor_hostname\u0027] \u003d\u003d nodename:"},{"line_number":188,"context_line":"                    db_compute \u003d compute"},{"line_number":189,"context_line":"                    # We can avoid an extra call to Service object in"},{"line_number":190,"context_line":"                    # _from_db_object"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_406c73d0","line":187,"in_reply_to":"3a961159_d1411ba7","updated":"2015-01-07 22:23:17.000000000","message":"There are never multiple compute nodes for a given service in our current implementation. Even if there were multiple compute node records, there would never be more than 1 compute node record returned by the compute_node_get_by_service_id() DB API method before, so there is no need to test it. All this patch is doing is uncrufting the previous stuff.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144db5da87503264353de0e9356cad3fbe39dfac","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":185,"context_line":"            db_compute \u003d None"},{"line_number":186,"context_line":"            for compute in db_computes:"},{"line_number":187,"context_line":"                if compute[\u0027hypervisor_hostname\u0027] \u003d\u003d nodename:"},{"line_number":188,"context_line":"                    db_compute \u003d compute"},{"line_number":189,"context_line":"                    # We can avoid an extra call to Service object in"},{"line_number":190,"context_line":"                    # _from_db_object"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_1d3fc688","line":187,"in_reply_to":"3a961159_e9b1cb01","updated":"2015-01-12 21:27:29.000000000","message":"Matt, you\u0027re right, test is missing, adding it","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b758b4d2ec209097280ad0d456beb5351f319783","unresolved":false,"context_lines":[{"line_number":317,"context_line":"    @classmethod"},{"line_number":318,"context_line":"    def get_by_service(cls, context, service, use_slave\u003dFalse):"},{"line_number":319,"context_line":"        # NOTE(sbauza): use_slave is deprecated and should be removed in a new"},{"line_number":320,"context_line":"        # version"},{"line_number":321,"context_line":"        return cls._get_by_service(context, service.id, use_slave\u003duse_slave)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_14d8f151","line":320,"updated":"2015-01-07 20:22:35.000000000","message":"Because of this change? Why?","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"33e425beee4d83646a296d46d9a9b61653cc212a","unresolved":false,"context_lines":[{"line_number":317,"context_line":"    @classmethod"},{"line_number":318,"context_line":"    def get_by_service(cls, context, service, use_slave\u003dFalse):"},{"line_number":319,"context_line":"        # NOTE(sbauza): use_slave is deprecated and should be removed in a new"},{"line_number":320,"context_line":"        # version"},{"line_number":321,"context_line":"        return cls._get_by_service(context, service.id, use_slave\u003duse_slave)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_40a393ff","line":320,"in_reply_to":"3a961159_14d8f151","updated":"2015-01-07 22:23:17.000000000","message":"The entire ComputeNode.get_by_service() method is deprecated. I don\u0027t know why bauzas was being specific about use_slave\u003dFalse.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"144db5da87503264353de0e9356cad3fbe39dfac","unresolved":false,"context_lines":[{"line_number":317,"context_line":"    @classmethod"},{"line_number":318,"context_line":"    def get_by_service(cls, context, service, use_slave\u003dFalse):"},{"line_number":319,"context_line":"        # NOTE(sbauza): use_slave is deprecated and should be removed in a new"},{"line_number":320,"context_line":"        # version"},{"line_number":321,"context_line":"        return cls._get_by_service(context, service.id, use_slave\u003duse_slave)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_dd28def0","line":320,"in_reply_to":"3a961159_2921134d","updated":"2015-01-12 21:27:29.000000000","message":"Done (forgot to remove from a previous PS)","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f564409124c502f16597f1c8c96939f72bf61e7b","unresolved":false,"context_lines":[{"line_number":317,"context_line":"    @classmethod"},{"line_number":318,"context_line":"    def get_by_service(cls, context, service, use_slave\u003dFalse):"},{"line_number":319,"context_line":"        # NOTE(sbauza): use_slave is deprecated and should be removed in a new"},{"line_number":320,"context_line":"        # version"},{"line_number":321,"context_line":"        return cls._get_by_service(context, service.id, use_slave\u003duse_slave)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_2921134d","line":320,"in_reply_to":"3a961159_40a393ff","updated":"2015-01-07 23:41:02.000000000","message":"Yeah, hence I\u0027d just nix this.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b758b4d2ec209097280ad0d456beb5351f319783","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host,"},{"line_number":336,"context_line":"                                                              use_slave)"},{"line_number":337,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":338,"context_line":"                    context, service.id)"},{"line_number":339,"context_line":"            except exception.ServiceNotFound:"},{"line_number":340,"context_line":"                # We need to provide the same exception upstream"},{"line_number":341,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_94914154","line":338,"updated":"2015-01-07 20:22:35.000000000","message":"You do this multiple times, where you query for the service and then the computes. Maybe we should just make a query that lets you do this in one hit?","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f564409124c502f16597f1c8c96939f72bf61e7b","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host,"},{"line_number":336,"context_line":"                                                              use_slave)"},{"line_number":337,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":338,"context_line":"                    context, service.id)"},{"line_number":339,"context_line":"            except exception.ServiceNotFound:"},{"line_number":340,"context_line":"                # We need to provide the same exception upstream"},{"line_number":341,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_a92d0348","line":338,"in_reply_to":"3a961159_80b01b44","updated":"2015-01-07 23:41:02.000000000","message":"Oh, right, because we won\u0027t have anything to join on going forward?","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"33e425beee4d83646a296d46d9a9b61653cc212a","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host,"},{"line_number":336,"context_line":"                                                              use_slave)"},{"line_number":337,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":338,"context_line":"                    context, service.id)"},{"line_number":339,"context_line":"            except exception.ServiceNotFound:"},{"line_number":340,"context_line":"                # We need to provide the same exception upstream"},{"line_number":341,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_80b01b44","line":338,"in_reply_to":"3a961159_94914154","updated":"2015-01-07 22:23:17.000000000","message":"We can\u0027t due to the fact that we are attempting to entirely deprecate the methods that related a compute node to a service.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"becbd1669ac346d04ac392aa394e494d03d6bd0a","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                service \u003d objects.Service.get_by_compute_host(context, host,"},{"line_number":336,"context_line":"                                                              use_slave)"},{"line_number":337,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":338,"context_line":"                    context, service.id)"},{"line_number":339,"context_line":"            except exception.ServiceNotFound:"},{"line_number":340,"context_line":"                # We need to provide the same exception upstream"},{"line_number":341,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_4cf0f466","line":338,"in_reply_to":"3a961159_a92d0348","updated":"2015-01-09 20:38:57.000000000","message":"I would refactor out the multiple calls into a static method here, and explain it once in that method\u0027s docstring. It does seem awkward to repeat the same workaround in multiple places.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07682bd8f2bc15d2c7476557c6382afc4b42769a","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                                                              use_slave)"},{"line_number":337,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":338,"context_line":"                    context, service.id)"},{"line_number":339,"context_line":"            except exception.ServiceNotFound:"},{"line_number":340,"context_line":"                # We need to provide the same exception upstream"},{"line_number":341,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":342,"context_line":"            # We can avoid an extra call to Service object in _from_db_object"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_718a27c0","line":339,"updated":"2015-01-07 20:04:07.000000000","message":"The DB API raises ComputeHostNotFound, not ServiceNotFound, so why are you catching this here?\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/db/sqlalchemy/api.py#n431\n\nOh because of the db.compute_node_get_by_service_id call:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/db/sqlalchemy/api.py#n497","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"33e425beee4d83646a296d46d9a9b61653cc212a","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                                                              use_slave)"},{"line_number":337,"context_line":"                db_computes \u003d db.compute_node_get_by_service_id("},{"line_number":338,"context_line":"                    context, service.id)"},{"line_number":339,"context_line":"            except exception.ServiceNotFound:"},{"line_number":340,"context_line":"                # We need to provide the same exception upstream"},{"line_number":341,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":342,"context_line":"            # We can avoid an extra call to Service object in _from_db_object"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_00842b5c","line":339,"in_reply_to":"3a961159_718a27c0","updated":"2015-01-07 22:23:17.000000000","message":"Correct.","commit_id":"aeb8cc8aa435774de5f32e09bf8d75657da04e54"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"043e40db277e3f869424b85cdb94594119f279b9","unresolved":false,"context_lines":[{"line_number":180,"context_line":"                db_computes \u003d db.compute_nodes_get_by_service_id("},{"line_number":181,"context_line":"                    context, service.id)"},{"line_number":182,"context_line":"            except exception.ServiceNotFound:"},{"line_number":183,"context_line":"                # We need to provide the same exception upstream"},{"line_number":184,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":185,"context_line":"            db_compute \u003d None"},{"line_number":186,"context_line":"            for compute in db_computes:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_6d2b3c33","line":183,"updated":"2015-01-15 19:44:37.000000000","message":"Why? Since this isn\u0027t bumping the version of ComputeNode, shouldn\u0027t it not change the behavior of the current version? IIUC, the current version can raise ComputeHostNotFound from the first line and ServiceNotFound from the second -- and does no conversion to ComputeHostNotFound.","commit_id":"027e7b64af7009f481c6835342b9ba451389bee0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"43eab90e19be1775345658880222239ab7e0d0e7","unresolved":false,"context_lines":[{"line_number":180,"context_line":"                db_computes \u003d db.compute_nodes_get_by_service_id("},{"line_number":181,"context_line":"                    context, service.id)"},{"line_number":182,"context_line":"            except exception.ServiceNotFound:"},{"line_number":183,"context_line":"                # We need to provide the same exception upstream"},{"line_number":184,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost)"},{"line_number":185,"context_line":"            db_compute \u003d None"},{"line_number":186,"context_line":"            for compute in db_computes:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_46072c1d","line":183,"in_reply_to":"3a961159_6d2b3c33","updated":"2015-01-16 10:03:53.000000000","message":"I agree with you that we change the behaviour but those classemethods are for the moment not in use, but have been added by a previous patch in the series. So I think it would be overkill to bump a new version just for this, as the goal of this patch is to fix those 2 broken unused methods.\n\nJust to be clear, those methods will be used in the next patches in the series","commit_id":"027e7b64af7009f481c6835342b9ba451389bee0"}]}