)]}'
{"nova/api/openstack/compute/contrib/flavor_access.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"33198994f239d0b96969413ae94cc7ff50270308","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        tenant \u003d vals[\u0027tenant\u0027]"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        flavor \u003d flavor_obj.Flavor.get_by_flavor_id(context, id)"},{"line_number":173,"context_line":"        if tenant in flavor.projects:"},{"line_number":174,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":175,"context_line":"                explanation\u003d_(\u0027flavor is already allowed for tenant\u0027))"},{"line_number":176,"context_line":"        flavor.projects.append(tenant)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BzXc%3D","line":173,"updated":"2013-11-19 23:45:37.000000000","message":"Bummer... this is adding 2 additional DB queries over the current code.  I wonder if we should have a add_flavor_access() classmethod, instead.\n\nRe-reviewing save() from the previous patch, it looks like this code is actually going to cause 4 additional DB queries.  Because save() is also going to do similar logic.","commit_id":"8d44f94d0576942fe01f0b628325244862ad54dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"17a311dd1389503aa2736ea432992d3c9aa2b1e2","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        tenant \u003d vals[\u0027tenant\u0027]"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        flavor \u003d flavor_obj.Flavor.get_by_flavor_id(context, id)"},{"line_number":173,"context_line":"        if tenant in flavor.projects:"},{"line_number":174,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":175,"context_line":"                explanation\u003d_(\u0027flavor is already allowed for tenant\u0027))"},{"line_number":176,"context_line":"        flavor.projects.append(tenant)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Br84%3D","line":173,"in_reply_to":"AAAATn%2F%2BzXc%3D","updated":"2013-11-21 14:56:06.000000000","message":"Yes, until the unified DB helper is added. I kinda feel like adding a classmethod for everything we want to do out of body is not really improving the flow over just the purely functional layout we have right now. In other cases, I have done that because the calls were likely to be in the critical path, but this is just management, which doesn\u0027t happen all that often, right?\n\nOn the other hand, we\u0027re trying to make things better not worse, so if you think it\u0027s important to wrap this all in a single classmethod, then we should do that.","commit_id":"8d44f94d0576942fe01f0b628325244862ad54dc"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"33198994f239d0b96969413ae94cc7ff50270308","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                explanation\u003d_(\u0027flavor not currently allowed for tenant\u0027))"},{"line_number":200,"context_line":"        flavor.projects.remove(tenant)"},{"line_number":201,"context_line":"        try:"},{"line_number":202,"context_line":"            flavor.save()"},{"line_number":203,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":204,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":205,"context_line":"                explanation\u003derr.format_message())"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BzXI%3D","line":202,"updated":"2013-11-19 23:45:37.000000000","message":"Hm, save.","commit_id":"8d44f94d0576942fe01f0b628325244862ad54dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43a4d07cecd69b7c391f31d035a4878b2c6180c5","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                explanation\u003d_(\u0027flavor not currently allowed for tenant\u0027))"},{"line_number":200,"context_line":"        flavor.projects.remove(tenant)"},{"line_number":201,"context_line":"        try:"},{"line_number":202,"context_line":"            flavor.save()"},{"line_number":203,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":204,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":205,"context_line":"                explanation\u003derr.format_message())"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2Bpy8%3D","line":202,"updated":"2013-11-21 23:49:41.000000000","message":"So, after writing all of this, I realized that we have the potential to roll back the universe if we maintain any sort of list in the object and make it take effect in a .save() sort of operation.\n\nI submitted it anyway for discussion, but it seems like a classmethod for add and remove will be necessary to avoid this, I just wanted to get some confirmation before I go do it :(","commit_id":"dc8f1fbd3ddd2c6216bae7a72464deb13cdd6b2b"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"d90e9a389ef93892bb8ada1c529a62b03a325725","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                explanation\u003d_(\u0027flavor not currently allowed for tenant\u0027))"},{"line_number":200,"context_line":"        flavor.projects.remove(tenant)"},{"line_number":201,"context_line":"        try:"},{"line_number":202,"context_line":"            flavor.save()"},{"line_number":203,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":204,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":205,"context_line":"                explanation\u003derr.format_message())"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2Bpmk%3D","line":202,"in_reply_to":"AAAATn%2F%2Bpy8%3D","updated":"2013-11-22 00:49:19.000000000","message":"Kinda doing drive by reviews with a headache, so I\u0027m not sure I get what you mean. :)   What does \u0027roll back the universe\u0027 mean?    Got an example?","commit_id":"dc8f1fbd3ddd2c6216bae7a72464deb13cdd6b2b"},{"author":{"_account_id":2468,"name":"unicell","email":"unicell@gmail.com","username":"unicell"},"change_message_id":"784d81d15df94eefab19772446221590b95b4310","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        except (exception.FlavorNotFound,"},{"line_number":198,"context_line":"                exception.FlavorAccessNotFound) as err:"},{"line_number":199,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003derr.format_message())"},{"line_number":200,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":201,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003derr.format_message())"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        return _marshall_flavor_access(flavor)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2BWIc%3D","line":200,"updated":"2013-11-27 04:49:27.000000000","message":"Sorry, but in which case would flavor.remove_access() throws up a FlavorAccessExists exception? I don\u0027t see any.","commit_id":"d2caeb01fbf1bab9136ad7e223747c477e946d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"90dd530c5a25f11d5dcea9e63391d3e37cec1062","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _extend_flavor(self, flavor_rval, flavor_ref):"},{"line_number":125,"context_line":"        key \u003d \"%s:is_public\" % (Flavor_access.alias)"},{"line_number":126,"context_line":"        flavor_rval[key] \u003d flavor_ref[\u0027is_public\u0027]"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    @wsgi.extends"},{"line_number":129,"context_line":"    def show(self, req, resp_obj, id):"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9ybk%3D","line":126,"updated":"2013-12-05 22:13:04.000000000","message":"Is converting flavor_ref to an object meant for a later patch?","commit_id":"3500d8d3e582a4dc1a561b1003e2aff79fda2107"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b0362b3c3b2020589f2fd865626f50eb064a1d3c","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _extend_flavor(self, flavor_rval, flavor_ref):"},{"line_number":125,"context_line":"        key \u003d \"%s:is_public\" % (Flavor_access.alias)"},{"line_number":126,"context_line":"        flavor_rval[key] \u003d flavor_ref[\u0027is_public\u0027]"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    @wsgi.extends"},{"line_number":129,"context_line":"    def show(self, req, resp_obj, id):"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9uwc%3D","line":126,"in_reply_to":"AAAATn%2F9ybk%3D","updated":"2013-12-06 16:18:56.000000000","message":"flavor_ref is still a dict here because it came out of the cache, which isn\u0027t storing an object.","commit_id":"3500d8d3e582a4dc1a561b1003e2aff79fda2107"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"90dd530c5a25f11d5dcea9e63391d3e37cec1062","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":176,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003derr.format_message())"},{"line_number":177,"context_line":"        except (exception.FlavorNotFound,"},{"line_number":178,"context_line":"                exception.FlavorAccessNotFound) as err:"},{"line_number":179,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003derr.format_message())"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        return _marshall_flavor_access(flavor)"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9ycU%3D","line":178,"updated":"2013-12-05 22:13:04.000000000","message":"Why would we get FlavorAccessNotFound here?  Looks like that only shows up on db_api.flavor_access_remove.\n\nIf there is a reason to add this check, we should have a test case that covers it.","commit_id":"3500d8d3e582a4dc1a561b1003e2aff79fda2107"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"90dd530c5a25f11d5dcea9e63391d3e37cec1062","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        except (exception.FlavorNotFound,"},{"line_number":198,"context_line":"                exception.FlavorAccessNotFound) as err:"},{"line_number":199,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003derr.format_message())"},{"line_number":200,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":201,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003derr.format_message())"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        return _marshall_flavor_access(flavor)"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9ydc%3D","line":200,"updated":"2013-12-05 22:13:04.000000000","message":"Why would we get a FlavorAccessExists when we\u0027re removing a flavor?  Looks like that only shows up on db_api.flavor_access_add.\n\nAlso, you don\u0027t have this same change in the v3 API.\n\nIf there is a reason to add this check, we should have a test case that covers it.","commit_id":"3500d8d3e582a4dc1a561b1003e2aff79fda2107"}],"nova/api/openstack/compute/plugins/v3/flavor_access.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b97c8b62cdb0fd6497ed1f52758b9fad718c6315","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                explanation\u003d_(\"tenant_id is required\"))"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        try:"},{"line_number":174,"context_line":"            flavor \u003d flavor_obj.Flavor.get_by_flavor_id(context, id)"},{"line_number":175,"context_line":"        except exception.FlavorNotFound as e:"},{"line_number":176,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B%2FVI%3D","line":174,"updated":"2013-11-16 07:02:31.000000000","message":"Look through the code. In flavor.save(), it will invoke \u0027db.flavor_access_add\u0027. Then in \u0027db.flavor_access_add\u0027 will access db to get flavor again. Is there any chance to avoid access db double times?","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"595a37d412bf3830fe3e114783d254364d861c2b","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                explanation\u003d_(\"tenant_id is required\"))"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        try:"},{"line_number":174,"context_line":"            flavor \u003d flavor_obj.Flavor.get_by_flavor_id(context, id)"},{"line_number":175,"context_line":"        except exception.FlavorNotFound as e:"},{"line_number":176,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B7EM%3D","line":174,"in_reply_to":"AAAATn%2F%2B%2FVI%3D","updated":"2013-11-18 15:05:52.000000000","message":"Yes, there is definitely opportunity to optimize those paths. They need to (IMHO) be refactored quite a bit anyway. Since this is just a management extension and not in the critical path, I think it\u0027s worth getting everything on the new data model and then fixing the underlying bits which will improve everything.","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b97c8b62cdb0fd6497ed1f52758b9fad718c6315","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        except exception.FlavorNotFound as e:"},{"line_number":176,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        if tenant in flavor.projects:"},{"line_number":179,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":180,"context_line":"                explanation\u003d_(\u0027flavor is already allowed for tenant\u0027))"},{"line_number":181,"context_line":"        flavor.projects.append(tenant)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B%2FVY%3D","line":178,"updated":"2013-11-16 07:02:31.000000000","message":"Maybe we needn\u0027t doing this check at here. Because flavor.save() will have same checking.","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"79eb4c78d69289b4ecc3d511bdb3297301a96308","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        except exception.FlavorNotFound as e:"},{"line_number":176,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        if tenant in flavor.projects:"},{"line_number":179,"context_line":"            raise webob.exc.HTTPConflict("},{"line_number":180,"context_line":"                explanation\u003d_(\u0027flavor is already allowed for tenant\u0027))"},{"line_number":181,"context_line":"        flavor.projects.append(tenant)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B%2FUs%3D","line":178,"in_reply_to":"AAAATn%2F%2B%2FVY%3D","updated":"2013-11-16 07:15:43.000000000","message":"oops, sorry, missunderstand the code, please ignore this comment.","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b97c8b62cdb0fd6497ed1f52758b9fad718c6315","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        try:"},{"line_number":184,"context_line":"            flavor.save()"},{"line_number":185,"context_line":"        except exception.FlavorAccessExists as err:"},{"line_number":186,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003derr.format_message())"},{"line_number":187,"context_line":"        except exception.AdminRequired as e:"},{"line_number":188,"context_line":"            raise webob.exc.HTTPForbidden(explanation\u003de.format_message())"},{"line_number":189,"context_line":"        return _marshall_flavor_access(flavor)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B%2FVk%3D","line":186,"updated":"2013-11-16 07:02:31.000000000","message":"also need catch FlavorNotFound at here. There is chance for flavor deleted before save.","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b97c8b62cdb0fd6497ed1f52758b9fad718c6315","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        except exception.FlavorNotFound as e:"},{"line_number":211,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        if tenant not in flavor.projects:"},{"line_number":214,"context_line":"            raise webob.exc.HTTPNotFound("},{"line_number":215,"context_line":"                explanation\u003d_(\u0027flavor not currently allowed for tenant\u0027))"},{"line_number":216,"context_line":"        flavor.projects.remove(tenant)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B%2FVE%3D","line":213,"updated":"2013-11-16 07:02:31.000000000","message":"same as above.","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"79eb4c78d69289b4ecc3d511bdb3297301a96308","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        except exception.FlavorNotFound as e:"},{"line_number":211,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        if tenant not in flavor.projects:"},{"line_number":214,"context_line":"            raise webob.exc.HTTPNotFound("},{"line_number":215,"context_line":"                explanation\u003d_(\u0027flavor not currently allowed for tenant\u0027))"},{"line_number":216,"context_line":"        flavor.projects.remove(tenant)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2B%2FUo%3D","line":213,"in_reply_to":"AAAATn%2F%2B%2FVE%3D","updated":"2013-11-16 07:15:43.000000000","message":"ignore this too.","commit_id":"48ba34db72bdba0966ec90604a302826f4295d26"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"90dd530c5a25f11d5dcea9e63391d3e37cec1062","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        flavor \u003d flavor_obj.Flavor(context\u003dcontext, flavorid\u003did)"},{"line_number":174,"context_line":"        try:"},{"line_number":175,"context_line":"            flavor.add_access(tenant)"},{"line_number":176,"context_line":"        except (exception.FlavorAccessNotFound,"},{"line_number":177,"context_line":"                exception.FlavorNotFound) as e:"},{"line_number":178,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":179,"context_line":"        except exception.FlavorAccessExists as err:"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F9yas%3D","line":176,"updated":"2013-12-05 22:13:04.000000000","message":"Same question as v2, not sure why FlavorAccessNotFound is here.","commit_id":"3500d8d3e582a4dc1a561b1003e2aff79fda2107"}]}