)]}'
{"nova/db/sqlalchemy/resource_class_cache.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3500ebd96b46f28ae839fcb99d1c6961405a3655","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                 classes or the resource_classes database table."},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        if rc_id in self.str_cache:"},{"line_number":89,"context_line":"            return self.str_cache[rc_id]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # First check the fields.ResourceClass.ALL enum"},{"line_number":92,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_c03b529b","line":89,"updated":"2016-10-10 19:57:00.000000000","message":"Are we sure that deleting the last resource class and then posting a new resource class would have a different DB ID ?\n\nIf not, I\u0027d prefer to see the _refresh_from_db() method being not private so we could call it from the object method.","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"a01082ecc5ecac485d14644c19a4783ee9017827","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                 classes or the resource_classes database table."},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        if rc_id in self.str_cache:"},{"line_number":89,"context_line":"            return self.str_cache[rc_id]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # First check the fields.ResourceClass.ALL enum"},{"line_number":92,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_8fca544e","line":89,"in_reply_to":"fa6399be_8e550b1f","updated":"2016-10-11 09:26:05.000000000","message":"One case I guess is at here https://review.openstack.org/#/c/379656/9/nova/objects/resource_provider.py@866\n\nAlthough it isn\u0027t critical for user.","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"41ac293e530239492efb2b68385b378d91d32e9f","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                 classes or the resource_classes database table."},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        if rc_id in self.str_cache:"},{"line_number":89,"context_line":"            return self.str_cache[rc_id]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # First check the fields.ResourceClass.ALL enum"},{"line_number":92,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_e0b90f30","line":89,"in_reply_to":"fa6399be_8fca544e","updated":"2016-10-11 14:50:37.000000000","message":"I don\u0027t think so. I commented over there.","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1bedcd3a21b28581178444acff661cb298943454","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                 classes or the resource_classes database table."},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        if rc_id in self.str_cache:"},{"line_number":89,"context_line":"            return self.str_cache[rc_id]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # First check the fields.ResourceClass.ALL enum"},{"line_number":92,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_8e550b1f","line":89,"in_reply_to":"fa6399be_c03b529b","updated":"2016-10-10 21:14:45.000000000","message":"That\u0027s how auto-increment works, so yeah, I think we\u0027re fine. If we introduce some sort of data store later that behaves weirdly then we could change this, but the whole point of this is query-once.","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"40b64c0450383e86ebbfca0fbb28fdde50956aa3","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                 classes or the resource_classes database table."},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        if rc_id in self.str_cache:"},{"line_number":89,"context_line":"            return self.str_cache[rc_id]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # First check the fields.ResourceClass.ALL enum"},{"line_number":92,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_2b275165","line":89,"in_reply_to":"fa6399be_e0b90f30","updated":"2016-10-12 05:21:46.000000000","message":"replied over there also. also agree we can improve those later.","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"b4bed14a7f74126c4ea1e9454a7e7316bfa64338","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            return self.id_cache[rc_str]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # First check the standard resource classes"},{"line_number":68,"context_line":"        if rc_str in fields.ResourceClass.ALL:"},{"line_number":69,"context_line":"            return fields.ResourceClass.ALL.index(rc_str)"},{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            # Otherwise, check the database table"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_62e89f7d","line":68,"range":{"start_line":68,"start_character":8,"end_line":68,"end_character":46},"updated":"2016-10-11 17:20:05.000000000","message":"Shouldn\u0027t that be the consistent with code in line 92, so it can be like:\n\n    try:\n        return fields.ResourceClass.ALL.index(rc_str)\n    except ValueError:\n        _refresh_from_db(self.ctx, self)\n        return self.id_cache.get(rc_str)\n?","commit_id":"c46b469eb731287368081b719cbf30423b9e2d4c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"01a33fa44fe6d506098ed127e946f0278fe35dd6","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            return self.id_cache[rc_str]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # First check the standard resource classes"},{"line_number":68,"context_line":"        if rc_str in fields.ResourceClass.ALL:"},{"line_number":69,"context_line":"            return fields.ResourceClass.ALL.index(rc_str)"},{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            # Otherwise, check the database table"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_588452d3","line":68,"range":{"start_line":68,"start_character":8,"end_line":68,"end_character":46},"in_reply_to":"fa6399be_62e89f7d","updated":"2016-10-12 10:38:25.000000000","message":"Yeah, but no need to block on that really.","commit_id":"c46b469eb731287368081b719cbf30423b9e2d4c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"01a33fa44fe6d506098ed127e946f0278fe35dd6","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            return fields.ResourceClass.ALL.index(rc_str)"},{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            # Otherwise, check the database table"},{"line_number":72,"context_line":"            _refresh_from_db(self.ctx, self)"},{"line_number":73,"context_line":"            return self.id_cache.get(rc_str)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def string_from_id(self, rc_id):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_535993fd","line":72,"updated":"2016-10-12 10:38:25.000000000","message":"So if you ask for something not in the DB, we don\u0027t cache that non-existence, and we force a refresh from the DB, but I suspect thats probably not a big deal?","commit_id":"c46b469eb731287368081b719cbf30423b9e2d4c"}],"nova/objects/resource_class_cache.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dc7b4d8c5c736d3d178aaa43c73a45b2825beb1b","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        res \u003d conn.execute(sel).fetchall()"},{"line_number":27,"context_line":"        if not res:"},{"line_number":28,"context_line":"            return None"},{"line_number":29,"context_line":"        return res[0][0]"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"@db_api.api_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_3ddc7e72","line":29,"updated":"2016-10-07 16:56:21.000000000","message":"Is there any reason not to just load these all in one go the first time we need them? I guess we could have thousands of types, but .. I dunno, seems like it might be most useful to just select them all and avoid the hits after we\u0027ve done that. Am I being silly?","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"95830bc145dccde98f15648984e6c6c0379331b1","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        res \u003d conn.execute(sel).fetchall()"},{"line_number":27,"context_line":"        if not res:"},{"line_number":28,"context_line":"            return None"},{"line_number":29,"context_line":"        return res[0][0]"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"@db_api.api_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_9d05927b","line":29,"in_reply_to":"1a95cdbc_3ddc7e72","updated":"2016-10-07 17:04:48.000000000","message":"Yes, I thought about that. We\u0027d need to replace the entire cache when a new custom resource class was created (since the cache miss would fetch all the resource classes) but that\u0027s OK I guess. I could go either way. :)","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e3d99d6e6fc5dfd75e43f11228d1bda92c9f393e","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        res \u003d conn.execute(sel).fetchall()"},{"line_number":27,"context_line":"        if not res:"},{"line_number":28,"context_line":"            return None"},{"line_number":29,"context_line":"        return res[0][0]"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"@db_api.api_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_1db26294","line":29,"in_reply_to":"1a95cdbc_9d05927b","updated":"2016-10-07 17:07:19.000000000","message":"Yeah, we\u0027d still fault if we need to check about one we don\u0027t know about, for sure. But, see my example in the next patch about why it might be better to do them all (or at least, limit\u003d100 to try to pre-load a reasonable amount on the first hit).","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dc7b4d8c5c736d3d178aaa43c73a45b2825beb1b","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        return res[0][0]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class ResourceClassCache(object):"},{"line_number":43,"context_line":"    \"\"\"A cache of integer and string lookup values for resource classes.\"\"\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, ctx):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_9d9cb2c1","line":42,"updated":"2016-10-07 16:56:21.000000000","message":"Shouldn\u0027t this really behave like a singleton?","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e3d99d6e6fc5dfd75e43f11228d1bda92c9f393e","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        return res[0][0]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class ResourceClassCache(object):"},{"line_number":43,"context_line":"    \"\"\"A cache of integer and string lookup values for resource classes.\"\"\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, ctx):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_5da86a20","line":42,"in_reply_to":"1a95cdbc_1d6fc22b","updated":"2016-10-07 17:07:19.000000000","message":"Just to avoid ending up with two caches and more hits to the db than we need because we don\u0027t realize it.","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"95830bc145dccde98f15648984e6c6c0379331b1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        return res[0][0]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class ResourceClassCache(object):"},{"line_number":43,"context_line":"    \"\"\"A cache of integer and string lookup values for resource classes.\"\"\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, ctx):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_1d6fc22b","line":42,"in_reply_to":"1a95cdbc_9d9cb2c1","updated":"2016-10-07 17:04:48.000000000","message":"Meh, that\u0027s an implementation detail. I can add a __new__() implementation that enforces a single per-process cache of resource classes. Makes testing harder, though, which is why I just kept it like this and enforce the singleton-ness in the nova/objects/resource_provider.py file\u0027s _ensure_rc_cache() method.","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dc7b4d8c5c736d3d178aaa43c73a45b2825beb1b","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.id_cache \u003d {}"},{"line_number":53,"context_line":"        self.str_cache \u003d {}"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def id_from_string(self, rc_str):"},{"line_number":56,"context_line":"        \"\"\"Given a string representation of a resource class -- e.g. \"DISK_GB\""},{"line_number":57,"context_line":"        or \"IRON_SILVER\" -- return the integer code for the resource class. For"},{"line_number":58,"context_line":"        standard resource classes, this integer code will match the list of"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_bd3dee90","line":55,"updated":"2016-10-07 16:56:21.000000000","message":"These are not callable from the compute nodes, you know. Is that intentional? Do you never expect them to need to know the integer IDs? Maybe that\u0027s good and by design, but..","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"95830bc145dccde98f15648984e6c6c0379331b1","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.id_cache \u003d {}"},{"line_number":53,"context_line":"        self.str_cache \u003d {}"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def id_from_string(self, rc_str):"},{"line_number":56,"context_line":"        \"\"\"Given a string representation of a resource class -- e.g. \"DISK_GB\""},{"line_number":57,"context_line":"        or \"IRON_SILVER\" -- return the integer code for the resource class. For"},{"line_number":58,"context_line":"        standard resource classes, this integer code will match the list of"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_dd571a58","line":55,"in_reply_to":"1a95cdbc_bd3dee90","updated":"2016-10-07 17:04:48.000000000","message":"Yes, integer IDs are only ever applicable on the server side of the placement API.","commit_id":"9bbbdb8f624e9d495a87189dbf8d48910edebb9a"}],"nova/tests/functional/db/test_resource_class_cache.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3500ebd96b46f28ae839fcb99d1c6961405a3655","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        self.assertEqual(\u0027VCPU\u0027, cache.string_from_id(0))"},{"line_number":39,"context_line":"        self.assertEqual(\u0027MEMORY_MB\u0027, cache.string_from_id(1))"},{"line_number":40,"context_line":"        self.assertEqual(0, cache.id_from_string(\u0027VCPU\u0027))"},{"line_number":41,"context_line":"        self.assertEqual(1, cache.id_from_string(\u0027MEMORY_MB\u0027))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        self.assertFalse(sel_mock.called)"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_2095ee3f","line":41,"updated":"2016-10-10 19:57:00.000000000","message":"I\u0027d prefer to see a mock getting the DB records instead of using the functional test for getting what\u0027s already in the DB. \n\nIf we create a next DB record that\u0027s not custom, then we would need to modify that test, which could be confusing instead of just leaving the mock unchanged.","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1bedcd3a21b28581178444acff661cb298943454","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        self.assertEqual(\u0027VCPU\u0027, cache.string_from_id(0))"},{"line_number":39,"context_line":"        self.assertEqual(\u0027MEMORY_MB\u0027, cache.string_from_id(1))"},{"line_number":40,"context_line":"        self.assertEqual(0, cache.id_from_string(\u0027VCPU\u0027))"},{"line_number":41,"context_line":"        self.assertEqual(1, cache.id_from_string(\u0027MEMORY_MB\u0027))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        self.assertFalse(sel_mock.called)"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_6e06f721","line":41,"in_reply_to":"fa6399be_2095ee3f","updated":"2016-10-10 21:14:45.000000000","message":"He has mocked the DB here and is making sure we _don\u0027t_ hit it for the base classes. What other mock do you want?","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3500ebd96b46f28ae839fcb99d1c6961405a3655","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        with mock.patch(\u0027sqlalchemy.select\u0027) as sel_mock:"},{"line_number":69,"context_line":"            self.assertEqual(\u0027IRON_NFV\u0027, cache.string_from_id(1001))"},{"line_number":70,"context_line":"            self.assertEqual(1001, cache.id_from_string(\u0027IRON_NFV\u0027))"},{"line_number":71,"context_line":"            self.assertFalse(sel_mock.called)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa6399be_60c44640","line":71,"updated":"2016-10-10 19:57:00.000000000","message":"Could we have some negative test that would make sure that if we don\u0027t have some right resource class name, it would lookup the DB and fail?","commit_id":"f36e85c3966ebdd21e61040ef0807787b9eb3508"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"01a33fa44fe6d506098ed127e946f0278fe35dd6","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        with mock.patch(\u0027sqlalchemy.select\u0027) as sel_mock:"},{"line_number":69,"context_line":"            self.assertEqual(\u0027IRON_NFV\u0027, cache.string_from_id(1001))"},{"line_number":70,"context_line":"            self.assertEqual(1001, cache.id_from_string(\u0027IRON_NFV\u0027))"},{"line_number":71,"context_line":"            self.assertFalse(sel_mock.called)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_f368e789","line":71,"updated":"2016-10-12 10:38:25.000000000","message":"-1 is really for not having a test for the \"its not in the DB, or the cache or the default list\" case.","commit_id":"c46b469eb731287368081b719cbf30423b9e2d4c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"61a487947089fe120f66df3ae47d6429c8c801d3","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        with mock.patch(\u0027sqlalchemy.select\u0027) as sel_mock:"},{"line_number":69,"context_line":"            self.assertEqual(\u0027IRON_NFV\u0027, cache.string_from_id(1001))"},{"line_number":70,"context_line":"            self.assertEqual(1001, cache.id_from_string(\u0027IRON_NFV\u0027))"},{"line_number":71,"context_line":"            self.assertFalse(sel_mock.called)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_b3f58fcc","line":71,"updated":"2016-10-12 10:38:44.000000000","message":"Could we have some negative test that would make sure that if we don\u0027t have some right resource class name, it would lookup the DB and fail?","commit_id":"c46b469eb731287368081b719cbf30423b9e2d4c"}]}