)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8a3097d793af1f22b75f706b9bdbefeb505ac26b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t use OVO for Allocation and AllocationList"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Continuing the experiment to see the impact of review OVO from"},{"line_number":10,"context_line":"intermediary (between the HTTP API and the DB) classes in placement."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This change turns Allocation and AllocationList into classical"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fdfeff1_2200fc01","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":53},"updated":"2019-02-15 21:53:14.000000000","message":"to remove","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c795c9e91da71df486ed78c588e4b7338f25fa2d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Continuing the experiment to see the impact of review OVO from"},{"line_number":10,"context_line":"intermediary (between the HTTP API and the DB) classes in placement."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This change turns Allocation and AllocationList into classical"},{"line_number":13,"context_line":"objects. In the process an unused context field is removed"},{"line_number":14,"context_line":"from creators of Allocations, of which there are many."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fdfeff1_829f51b2","line":12,"range":{"start_line":12,"start_character":33,"end_line":12,"end_character":47},"updated":"2019-02-18 12:11:36.000000000","message":"would be great to just use a list object eventually for all the XXXList ovo objects...","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"}],"placement/objects/resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8a3097d793af1f22b75f706b9bdbefeb505ac26b","unresolved":false,"context_lines":[{"line_number":2140,"context_line":"                    used\u003dalloc.used)"},{"line_number":2141,"context_line":"            res \u003d context.session.execute(ins_stmt)"},{"line_number":2142,"context_line":"            alloc.id \u003d res.lastrowid"},{"line_number":2143,"context_line":"            alloc.obj_reset_changes()"},{"line_number":2144,"context_line":""},{"line_number":2145,"context_line":"        # Generation checking happens here. If the inventory for this resource"},{"line_number":2146,"context_line":"        # provider changed out from under us, this will raise a"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_ada5bd04","side":"PARENT","line":2143,"range":{"start_line":2143,"start_character":12,"end_line":2143,"end_character":37},"updated":"2019-02-15 21:53:14.000000000","message":"Was this so that, theoretically, any auto-generated/-incremented/-updated/-whatever fields in the db would get flushed back out to the object? But we don\u0027t need it because the only thing that applies to is the `id`, which is being explicitly read out?\n\nUm, what about the timestamp fields?\n\n[Later] I think the only thing that saves us here is the fact that this is an insert, which means created_at \u003d\u003d updated_at \u003d\u003d pretty-much-now (as noted at [0]), which gets defaulted to python\u0027s idea of \u0027now\u0027 by [1] and/or [2]. Shouldn\u0027t we be more rigorous than that and pull those two fields from `res`?\n\n[0] https://github.com/openstack/placement/blob/530be3d3985f02e2eb7d45e6f2977b294eb9cc65/placement/handlers/allocation.py#L43-L45\n[1] https://github.com/openstack/placement/blob/530be3d3985f02e2eb7d45e6f2977b294eb9cc65/placement/util.py#L144-L147\n[2] https://github.com/openstack/placement/blob/530be3d3985f02e2eb7d45e6f2977b294eb9cc65/placement/handlers/allocation.py#L54","commit_id":"b54733af92e94e1b29448662f426dda50331662e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c795c9e91da71df486ed78c588e4b7338f25fa2d","unresolved":false,"context_lines":[{"line_number":2140,"context_line":"                    used\u003dalloc.used)"},{"line_number":2141,"context_line":"            res \u003d context.session.execute(ins_stmt)"},{"line_number":2142,"context_line":"            alloc.id \u003d res.lastrowid"},{"line_number":2143,"context_line":"            alloc.obj_reset_changes()"},{"line_number":2144,"context_line":""},{"line_number":2145,"context_line":"        # Generation checking happens here. If the inventory for this resource"},{"line_number":2146,"context_line":"        # provider changed out from under us, this will raise a"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_220c65ea","side":"PARENT","line":2143,"range":{"start_line":2143,"start_character":12,"end_line":2143,"end_character":37},"in_reply_to":"9fdfeff1_ada5bd04","updated":"2019-02-18 12:11:36.000000000","message":"\u003e idea of \u0027now\u0027 by [1] and/or [2]. Shouldn\u0027t we be more rigorous than\n \u003e that and pull those two fields from `res`?\n\nres doesn\u0027t contain the created_at or updated_at fields, though. It\u0027s a sqlalchemy ResultProxy object and that is used for getting things like the last inserted serial/autoinc field (as shown above) and the number of rows affected by the INSERT/UPDATE/DELETE statement.\n\nThe RowProxy object in sqlalchemy is what is used for referencing fields in records returned via the underlying dbapi.cursor (which isn\u0027t returned for INSERT/UPDATE/DELETE statements AFAIK)","commit_id":"b54733af92e94e1b29448662f426dda50331662e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"068b2811d04469b31e6d7722e82feb584a479d84","unresolved":false,"context_lines":[{"line_number":2140,"context_line":"                    used\u003dalloc.used)"},{"line_number":2141,"context_line":"            res \u003d context.session.execute(ins_stmt)"},{"line_number":2142,"context_line":"            alloc.id \u003d res.lastrowid"},{"line_number":2143,"context_line":"            alloc.obj_reset_changes()"},{"line_number":2144,"context_line":""},{"line_number":2145,"context_line":"        # Generation checking happens here. If the inventory for this resource"},{"line_number":2146,"context_line":"        # provider changed out from under us, this will raise a"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_10df3966","side":"PARENT","line":2143,"range":{"start_line":2143,"start_character":12,"end_line":2143,"end_character":37},"in_reply_to":"9fdfeff1_ada5bd04","updated":"2019-02-16 12:40:47.000000000","message":"created at and updated at are defaulted by the db schema and the db itself. Since allocations are never updated, only deleted or inserted your assessment is pretty much on, and we don\u0027t need to care about it here because we do not return allocations on writes (by very intentional design).\n\nHowever, where it might make sense to improve things, but is a separate bug that\u0027s been there for a while: get_all_by_consumer_id and get_all_by_resource_provider do not reference the updated_at and created_at columns when creating their results.\n\n(What obj_reset_changes does is say \"make it so this object no longer appears like it has any fields that have changed\", it doesn\u0027t change the values, only the metadata in ovo that says \"things have changed\". That metadata is crux of the issue with some of the updates issues later in the stack and the SETTABLE_FIELDS thing in ResourceProvider change.)","commit_id":"b54733af92e94e1b29448662f426dda50331662e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8a3097d793af1f22b75f706b9bdbefeb505ac26b","unresolved":false,"context_lines":[{"line_number":2071,"context_line":"    # been a resource provider (not consumer) generation coflict."},{"line_number":2072,"context_line":"    RP_CONFLICT_RETRY_COUNT \u003d 10"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"    def __init__(self, context, objects\u003dNone):"},{"line_number":2075,"context_line":"        self._context \u003d context"},{"line_number":2076,"context_line":"        self.objects \u003d objects or []"},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"    def __len__(self):"},{"line_number":2079,"context_line":"        \"\"\"List length is a proxy for truthiness.\"\"\""},{"line_number":2080,"context_line":"        return len(self.objects)"},{"line_number":2081,"context_line":""},{"line_number":2082,"context_line":"    def __getitem__(self, index):"},{"line_number":2083,"context_line":"        return self.objects[index]"},{"line_number":2084,"context_line":""},{"line_number":2085,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":2086,"context_line":"    @db_api.placement_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_4d145144","line":2083,"range":{"start_line":2074,"start_character":0,"end_line":2083,"end_character":34},"updated":"2019-02-15 21:53:14.000000000","message":"Yeah, I can see factoring this pattern out (later).","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b7b5ad038a855821fa1f639777bc702073a76b36","unresolved":false,"context_lines":[{"line_number":2071,"context_line":"    # been a resource provider (not consumer) generation coflict."},{"line_number":2072,"context_line":"    RP_CONFLICT_RETRY_COUNT \u003d 10"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"    def __init__(self, context, objects\u003dNone):"},{"line_number":2075,"context_line":"        self._context \u003d context"},{"line_number":2076,"context_line":"        self.objects \u003d objects or []"},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"    def __len__(self):"},{"line_number":2079,"context_line":"        \"\"\"List length is a proxy for truthiness.\"\"\""},{"line_number":2080,"context_line":"        return len(self.objects)"},{"line_number":2081,"context_line":""},{"line_number":2082,"context_line":"    def __getitem__(self, index):"},{"line_number":2083,"context_line":"        return self.objects[index]"},{"line_number":2084,"context_line":""},{"line_number":2085,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":2086,"context_line":"    @db_api.placement_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_9c3828a0","line":2083,"range":{"start_line":2074,"start_character":0,"end_line":2083,"end_character":34},"in_reply_to":"9fdfeff1_4d145144","updated":"2019-02-15 23:29:45.000000000","message":"FTFY [1] [2]. More to do, e.g. _set_objects, but it\u0027s a start.\n\n[1] https://review.openstack.org/#/c/637325/\n[2] https://review.openstack.org/#/c/637326/","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"068b2811d04469b31e6d7722e82feb584a479d84","unresolved":false,"context_lines":[{"line_number":2071,"context_line":"    # been a resource provider (not consumer) generation coflict."},{"line_number":2072,"context_line":"    RP_CONFLICT_RETRY_COUNT \u003d 10"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"    def __init__(self, context, objects\u003dNone):"},{"line_number":2075,"context_line":"        self._context \u003d context"},{"line_number":2076,"context_line":"        self.objects \u003d objects or []"},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"    def __len__(self):"},{"line_number":2079,"context_line":"        \"\"\"List length is a proxy for truthiness.\"\"\""},{"line_number":2080,"context_line":"        return len(self.objects)"},{"line_number":2081,"context_line":""},{"line_number":2082,"context_line":"    def __getitem__(self, index):"},{"line_number":2083,"context_line":"        return self.objects[index]"},{"line_number":2084,"context_line":""},{"line_number":2085,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":2086,"context_line":"    @db_api.placement_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_b0d3cd5f","line":2083,"range":{"start_line":2074,"start_character":0,"end_line":2083,"end_character":34},"in_reply_to":"9fdfeff1_9c3828a0","updated":"2019-02-16 12:40:47.000000000","message":"Fixed that for us, dear, us.","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c795c9e91da71df486ed78c588e4b7338f25fa2d","unresolved":false,"context_lines":[{"line_number":2071,"context_line":"    # been a resource provider (not consumer) generation coflict."},{"line_number":2072,"context_line":"    RP_CONFLICT_RETRY_COUNT \u003d 10"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"    def __init__(self, context, objects\u003dNone):"},{"line_number":2075,"context_line":"        self._context \u003d context"},{"line_number":2076,"context_line":"        self.objects \u003d objects or []"},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"    def __len__(self):"},{"line_number":2079,"context_line":"        \"\"\"List length is a proxy for truthiness.\"\"\""},{"line_number":2080,"context_line":"        return len(self.objects)"},{"line_number":2081,"context_line":""},{"line_number":2082,"context_line":"    def __getitem__(self, index):"},{"line_number":2083,"context_line":"        return self.objects[index]"},{"line_number":2084,"context_line":""},{"line_number":2085,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":2086,"context_line":"    @db_api.placement_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_e2c65d9a","line":2083,"range":{"start_line":2074,"start_character":0,"end_line":2083,"end_character":34},"in_reply_to":"9fdfeff1_b0d3cd5f","updated":"2019-02-18 12:11:36.000000000","message":"I\u0027d much rather see the complete demise of XXXList objects entirely and just use simple module-level functions that accept a normal Python list of some thing, along with a Context object, and do their thing.\n\nI\u0027ve always bristled at the ovo XXXList objects and the @staticmethod style of things.\n\nIn other words, I\u0027d love to just have:\n\n from placement.db import allocation\n ...\n\n allocs \u003d [\n     ...\n ]\n allocation.set_allocations(ctx, allocs)","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"90f2377b111f27701b1ee9e584fa6624dbb6b496","unresolved":false,"context_lines":[{"line_number":2071,"context_line":"    # been a resource provider (not consumer) generation coflict."},{"line_number":2072,"context_line":"    RP_CONFLICT_RETRY_COUNT \u003d 10"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"    def __init__(self, context, objects\u003dNone):"},{"line_number":2075,"context_line":"        self._context \u003d context"},{"line_number":2076,"context_line":"        self.objects \u003d objects or []"},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"    def __len__(self):"},{"line_number":2079,"context_line":"        \"\"\"List length is a proxy for truthiness.\"\"\""},{"line_number":2080,"context_line":"        return len(self.objects)"},{"line_number":2081,"context_line":""},{"line_number":2082,"context_line":"    def __getitem__(self, index):"},{"line_number":2083,"context_line":"        return self.objects[index]"},{"line_number":2084,"context_line":""},{"line_number":2085,"context_line":"    @oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":2086,"context_line":"    @db_api.placement_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_22af857d","line":2083,"range":{"start_line":2074,"start_character":0,"end_line":2083,"end_character":34},"in_reply_to":"9fdfeff1_e2c65d9a","updated":"2019-02-18 12:20:11.000000000","message":"I think we\u0027ve kind of moving that way, but incrementally. Here\u0027s we swipe out the ovo-ness, eric\u0027s patches later dry up the duplication, and as I say there make it clear that there\u0027s room for even more simplification. I agree that \"actual, normal, lists\" is a good endpoint for all this, but figured we could minimize risk by making lots of little changes rather than a big swipe.\n\nI\u0027d be, and I imagine Eric would be, very happy to see patches after Eric\u0027s that move things in the direction that you say. \n\nI personally think we should feel free to change the guts as much as we want as long as the HTTP layer keeps the same behavior and we don\u0027t negatively impact performance or comprehension.","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"068b2811d04469b31e6d7722e82feb584a479d84","unresolved":false,"context_lines":[{"line_number":2190,"context_line":"                    context, id\u003drec[\u0027user_id\u0027],"},{"line_number":2191,"context_line":"                    external_id\u003drec[\u0027user_external_id\u0027]))"},{"line_number":2192,"context_line":"            objs.append("},{"line_number":2193,"context_line":"                Allocation("},{"line_number":2194,"context_line":"                    id\u003drec[\u0027id\u0027], resource_provider\u003drp,"},{"line_number":2195,"context_line":"                    resource_class\u003d_RC_CACHE.string_from_id("},{"line_number":2196,"context_line":"                        rec[\u0027resource_class_id\u0027]),"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_f0bcf506","line":2193,"updated":"2019-02-16 12:40:47.000000000","message":"here is where we ought to have time fields","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"068b2811d04469b31e6d7722e82feb584a479d84","unresolved":false,"context_lines":[{"line_number":2228,"context_line":"        # ResourceProvider object below by looking at the resource provider"},{"line_number":2229,"context_line":"        # fields returned by _get_allocations_by_consumer_id()."},{"line_number":2230,"context_line":"        objs \u003d ["},{"line_number":2231,"context_line":"            Allocation("},{"line_number":2232,"context_line":"                id\u003drec[\u0027id\u0027],"},{"line_number":2233,"context_line":"                resource_provider\u003dResourceProvider("},{"line_number":2234,"context_line":"                    context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_10b8990e","line":2231,"updated":"2019-02-16 12:40:47.000000000","message":"and here","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"}],"placement/tests/functional/db/test_base.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8a3097d793af1f22b75f706b9bdbefeb505ac26b","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return alloc_list"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _make_allocation(self, inv_dict, alloc_dict):"},{"line_number":119,"context_line":"        alloc_dict \u003d copy.copy(alloc_dict)"},{"line_number":120,"context_line":"        rp \u003d self._create_provider(\u0027allocation_resource_provider\u0027)"},{"line_number":121,"context_line":"        disk_inv \u003d rp_obj.Inventory(context\u003dself.ctx,"},{"line_number":122,"context_line":"                resource_provider\u003drp, **inv_dict)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_4da451a6","line":119,"range":{"start_line":119,"start_character":8,"end_line":119,"end_character":42},"updated":"2019-02-15 21:53:14.000000000","message":"Ew. But yeah.","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"}],"placement/util.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8a3097d793af1f22b75f706b9bdbefeb505ac26b","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    \"\"\""},{"line_number":141,"context_line":"    try:"},{"line_number":142,"context_line":"        current_modified \u003d (obj.updated_at or obj.created_at)"},{"line_number":143,"context_line":"    except NotImplementedError:"},{"line_number":144,"context_line":"        # If updated_at is not implemented, we are looking at objects that"},{"line_number":145,"context_line":"        # have not come from the database, so \"now\" is the right modified"},{"line_number":146,"context_line":"        # time."}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_4dfbf177","line":143,"range":{"start_line":143,"start_character":4,"end_line":143,"end_character":31},"updated":"2019-02-15 21:53:14.000000000","message":"Aside, just something I happened to notice while reviewing: Eventually this `except` clause should either go away (so we actually raise, because if we called this method with an obj lacking those fields, it\u0027s a bug); or be changed to trap AttributeError instead.","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"dd1dd8234806385f3392b315a4ad3cc38bde12ae","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    \"\"\""},{"line_number":141,"context_line":"    try:"},{"line_number":142,"context_line":"        current_modified \u003d (obj.updated_at or obj.created_at)"},{"line_number":143,"context_line":"    except NotImplementedError:"},{"line_number":144,"context_line":"        # If updated_at is not implemented, we are looking at objects that"},{"line_number":145,"context_line":"        # have not come from the database, so \"now\" is the right modified"},{"line_number":146,"context_line":"        # time."}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_68cca388","line":143,"range":{"start_line":143,"start_character":4,"end_line":143,"end_character":31},"in_reply_to":"9fdfeff1_4dfbf177","updated":"2019-02-15 22:38:45.000000000","message":"Ah, I see you marked it later in the series [1] but haven\u0027t yet removed it in the top patch [2].\n\n[1] https://review.openstack.org/#/c/636806/3/placement/util.py\n[2] https://review.openstack.org/#/c/636807/3/placement/util.py@144","commit_id":"34a172174766a671616a1a565c988b5b8a8d2dfe"}]}