)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24958aea1c0cdf9a982d057e9eedc0f1f666e097","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Chris Dent \u003ccdent@anticdent.org\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-07-30 16:14:25 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optimize trait creation to use cache"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Comparing benchmarks of creating (repeated) resource classes with"},{"line_number":10,"context_line":"traits, it became clear that creating resource classes was about"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_dd236ca6","line":7,"range":{"start_line":7,"start_character":31,"end_line":7,"end_character":36},"updated":"2019-07-30 16:07:08.000000000","message":"This patch isn\u0027t using the cache","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1ad11fc9f2acd977a369623ca9c4a0b6aeb12112","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Chris Dent \u003ccdent@anticdent.org\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-07-30 16:14:25 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optimize trait creation to use cache"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Comparing benchmarks of creating (repeated) resource classes with"},{"line_number":10,"context_line":"traits, it became clear that creating resource classes was about"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_bd0b10d3","line":7,"range":{"start_line":7,"start_character":31,"end_line":7,"end_character":36},"in_reply_to":"7faddb67_dd236ca6","updated":"2019-07-30 16:11:13.000000000","message":"it is. It\u0027s using the AttributeCache, I\u0027ll make that more clear.","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24958aea1c0cdf9a982d057e9eedc0f1f666e097","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Comparing benchmarks of creating (repeated) resource classes with"},{"line_number":10,"context_line":"traits, it became clear that creating resource classes was about"},{"line_number":11,"context_line":"1/3rd faster. Comparing the code, they were very similar, however"},{"line_number":12,"context_line":"the resource class side was ordered to check the cache first before"},{"line_number":13,"context_line":"attempting to do a create(). The meant that in the fairly common"},{"line_number":14,"context_line":"case where a reource class already existed the database would be hit"},{"line_number":15,"context_line":"only once, instead of twice."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_dd584c32","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":54},"updated":"2019-07-30 16:07:08.000000000","message":"eh? What code are you referring to here?\n\n[Later] update_resource_class, the 1.7+ one, I guess.\n\nAgain, no cache involved.","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24958aea1c0cdf9a982d057e9eedc0f1f666e097","unresolved":false,"context_lines":[{"line_number":14,"context_line":"case where a reource class already existed the database would be hit"},{"line_number":15,"context_line":"only once, instead of twice."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This change updates the \"ensure trait\" routine to be the same as the"},{"line_number":18,"context_line":"\"ensure resource class\" routine: check that it is already there,"},{"line_number":19,"context_line":"first. Since it is best practice to always ensure trait or resource"},{"line_number":20,"context_line":"class before using a CUSTOM_ one, this is a good orientation."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_7dc6f89c","line":18,"range":{"start_line":17,"start_character":24,"end_line":18,"end_character":23},"updated":"2019-07-30 16:07:08.000000000","message":"IMO references to \"ensure *\" are confusing. (Didn\u0027t we used to have ensure_rc and ensure_trait somewhere??) Clearer would be \"PUT handlers for /traits/$name and /resource_classes/$name\" or similar.","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1ad11fc9f2acd977a369623ca9c4a0b6aeb12112","unresolved":false,"context_lines":[{"line_number":14,"context_line":"case where a reource class already existed the database would be hit"},{"line_number":15,"context_line":"only once, instead of twice."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This change updates the \"ensure trait\" routine to be the same as the"},{"line_number":18,"context_line":"\"ensure resource class\" routine: check that it is already there,"},{"line_number":19,"context_line":"first. Since it is best practice to always ensure trait or resource"},{"line_number":20,"context_line":"class before using a CUSTOM_ one, this is a good orientation."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7faddb67_7df918c6","line":18,"range":{"start_line":17,"start_character":24,"end_line":18,"end_character":23},"in_reply_to":"7faddb67_7dc6f89c","updated":"2019-07-30 16:11:13.000000000","message":"will do","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"23fa9aff42cd6515418dfa001b1b88c280632e9a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"before attempting to do a create(). The meant that in the fairly common"},{"line_number":14,"context_line":"case where a reource class already existed we could finish early."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This change updates the code so that the common idiom used in client"},{"line_number":17,"context_line":"of \"ensuring a trait\" with PUT /traits/$name routine to be the same"},{"line_number":18,"context_line":"as the \"ensuring a resource class\" with PUT /resource_classes/$name"},{"line_number":19,"context_line":"routine: check that it is already there, first. Since it is best"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7faddb67_7d2838e1","line":16,"range":{"start_line":16,"start_character":48,"end_line":16,"end_character":68},"updated":"2019-07-30 16:41:48.000000000","message":"ah, that clears it up, thank you.","commit_id":"b04c771a9ffdf8c0eb31522eb10fd4850b74ae00"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"23fa9aff42cd6515418dfa001b1b88c280632e9a","unresolved":false,"context_lines":[{"line_number":21,"context_line":"CUSTOM_ one, this is a good orientation."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Note that with the recent addition of the AttributeCache, these queries"},{"line_number":24,"context_line":"for get_by_name [2] go to the ResourceClass of Trait cache. However, at"},{"line_number":25,"context_line":"this stage in processing, the cache is empty and call to get_by_name will"},{"line_number":26,"context_line":"fill it."},{"line_number":27,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7faddb67_5d70dc08","line":24,"range":{"start_line":24,"start_character":47,"end_line":24,"end_character":58},"updated":"2019-07-30 16:41:48.000000000","message":"Okay, I see what you were talking about, and it\u0027s true for ResourceClass, but not for Trait, whose get_by_name goes straight to the db, no cache [1].\n\nI suppose we could fix that in a separate patch. The cache is guaranteed to be cold at this point, so it\u0027s a no-op, but we could do it anyway by the same philosophy that prompted our defensive .clear()s, \n\n[1] https://opendev.org/openstack/placement/src/commit/2f56f379a568d427ac9057086386074a7801b00c/placement/objects/trait.py#L93-L103","commit_id":"b04c771a9ffdf8c0eb31522eb10fd4850b74ae00"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"23fa9aff42cd6515418dfa001b1b88c280632e9a","unresolved":false,"context_lines":[{"line_number":21,"context_line":"CUSTOM_ one, this is a good orientation."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Note that with the recent addition of the AttributeCache, these queries"},{"line_number":24,"context_line":"for get_by_name [2] go to the ResourceClass of Trait cache. However, at"},{"line_number":25,"context_line":"this stage in processing, the cache is empty and call to get_by_name will"},{"line_number":26,"context_line":"fill it."},{"line_number":27,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7faddb67_1d7fc4e0","line":24,"range":{"start_line":24,"start_character":44,"end_line":24,"end_character":46},"updated":"2019-07-30 16:41:48.000000000","message":"or","commit_id":"b04c771a9ffdf8c0eb31522eb10fd4850b74ae00"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c5cbdad3218ccb6700a8afa1b890b037485cb84e","unresolved":false,"context_lines":[{"line_number":21,"context_line":"CUSTOM_ one, this is a good orientation."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Note that with the recent addition of the AttributeCache, these queries"},{"line_number":24,"context_line":"for get_by_name [2] go to the ResourceClass of Trait cache. However, at"},{"line_number":25,"context_line":"this stage in processing, the cache is empty and call to get_by_name will"},{"line_number":26,"context_line":"fill it."},{"line_number":27,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7faddb67_804b4729","line":24,"range":{"start_line":24,"start_character":47,"end_line":24,"end_character":58},"in_reply_to":"7faddb67_5d70dc08","updated":"2019-07-30 17:05:36.000000000","message":"Sigh. That was supposed to happen in https://review.opendev.org/#/c/672298/\n\nor at least in a followup to it...","commit_id":"b04c771a9ffdf8c0eb31522eb10fd4850b74ae00"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"08eb8b08ba2a5d7d007b7393a34fa6f3c2fb2a02","unresolved":false,"context_lines":[{"line_number":11,"context_line":"1/3rd faster. Comparing the code, they were very similar, however"},{"line_number":12,"context_line":"the resource class side was ordered to check for existence first"},{"line_number":13,"context_line":"before attempting to do a create(). This meant that in the fairly common"},{"line_number":14,"context_line":"case where a reource class already existed we could finish early."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This change updates the code so that the common idiom used in clients"},{"line_number":17,"context_line":"of \"ensuring a trait\" with PUT /traits/$name routine to be the same"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7faddb67_49667eef","line":14,"range":{"start_line":14,"start_character":13,"end_line":14,"end_character":20},"updated":"2019-08-06 06:30:26.000000000","message":"resource","commit_id":"baea7003603e6c957002d501fafe7f95896885d8"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"08eb8b08ba2a5d7d007b7393a34fa6f3c2fb2a02","unresolved":false,"context_lines":[{"line_number":28,"context_line":"A unit test is added to confirm that the \u0027put_trait\u0027 handler will look"},{"line_number":29,"context_line":"at get_by_name and then go back to assuming the trait exists if the"},{"line_number":30,"context_line":"create() fails because it already exists. The general behavior of"},{"line_number":31,"context_line":"whether a trait is is created or updated is confirmed by the gabbit"},{"line_number":32,"context_line":"tests in trait.yaml."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"[1] https://opendev.org/openstack/placement/src/commit/2f56f379a568d427ac9057086386074a7801b00c/placement/handlers/resource_class.py#L209"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7faddb67_0943a640","line":31,"range":{"start_line":31,"start_character":19,"end_line":31,"end_character":22},"updated":"2019-08-06 06:30:26.000000000","message":"It is a duplicate. Remove it.","commit_id":"baea7003603e6c957002d501fafe7f95896885d8"}],"placement/handlers/resource_class.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24958aea1c0cdf9a982d057e9eedc0f1f666e097","unresolved":false,"context_lines":[{"line_number":224,"context_line":"    try:"},{"line_number":225,"context_line":"        rc \u003d rc_obj.ResourceClass.get_by_name(context, name)"},{"line_number":226,"context_line":"    except exception.NotFound:"},{"line_number":227,"context_line":"        try:"},{"line_number":228,"context_line":"            rc \u003d rc_obj.ResourceClass(context, name\u003dname)"},{"line_number":229,"context_line":"            rc.create()"},{"line_number":230,"context_line":"            status \u003d 201"},{"line_number":231,"context_line":"        # We will not see ResourceClassCannotUpdateStandard because"},{"line_number":232,"context_line":"        # that was already caught when validating the {name}."},{"line_number":233,"context_line":"        except exception.ResourceClassExists:"},{"line_number":234,"context_line":"            # Someone just now created the class, so stick with 204"},{"line_number":235,"context_line":"            pass"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    req.response.status \u003d status"},{"line_number":238,"context_line":"    req.response.content_type \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_7d4b582a","line":235,"range":{"start_line":227,"start_character":0,"end_line":235,"end_character":16},"updated":"2019-07-30 16:07:08.000000000","message":"this","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"}],"placement/handlers/trait.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24958aea1c0cdf9a982d057e9eedc0f1f666e097","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        trait \u003d trait_obj.Trait(context, name\u003dname)"},{"line_number":86,"context_line":"        trait.create()"},{"line_number":87,"context_line":"        status \u003d 201"},{"line_number":88,"context_line":"    except exception.TraitExists:"},{"line_number":89,"context_line":"        # Something just created the trait"},{"line_number":90,"context_line":"        pass"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_7d13585d","line":88,"updated":"2019-07-30 16:07:08.000000000","message":"You\u0027re expecting this TraitExists to come from trait.create() on L86, yah? That won\u0027t work; the scope of this `except` is only the `try` on L83:\n\n In [6]: try:\n   ...:     raise ValueError()\n   ...: except ValueError:\n   ...:     raise TypeError(\u0027foo\u0027)\n   ...: except TypeError as t:\n   ...:     print(\"Got here\")\n   ...:     \n ---------------------------------------------------------------------------\n TypeError                                 Traceback (most recent call last)\n \u003cipython-input-6-b425f914d627\u003e in \u003cmodule\u003e()\n       2     raise ValueError()\n       3 except ValueError:\n ----\u003e 4     raise TypeError(\u0027foo\u0027)\n       5 except TypeError as t:\n       6     print(\"Got here\")\n\n TypeError: foo\n\nThe thing you\u0027re mimicking [1] does it correctly, an extra nested try/except within the except *NotFound block.\n\n[1] https://review.opendev.org/#/c/673555/1/placement/handlers/resource_class.py@235","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1ad11fc9f2acd977a369623ca9c4a0b6aeb12112","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        trait \u003d trait_obj.Trait(context, name\u003dname)"},{"line_number":86,"context_line":"        trait.create()"},{"line_number":87,"context_line":"        status \u003d 201"},{"line_number":88,"context_line":"    except exception.TraitExists:"},{"line_number":89,"context_line":"        # Something just created the trait"},{"line_number":90,"context_line":"        pass"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_fd0508c3","line":88,"in_reply_to":"7faddb67_7d13585d","updated":"2019-07-30 16:11:13.000000000","message":"that\u0027s a typo","commit_id":"9111928f1a6b0113504d49dd5c7665e4fcf7395a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"656d25f2a9883972f3a6ff6118f9f24791925f84","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    req.response.content_type \u003d None"},{"line_number":95,"context_line":"    req.response.location \u003d util.trait_url(req.environ, trait)"},{"line_number":96,"context_line":"    if want_version.matches((1, 15)):"},{"line_number":97,"context_line":"        req.response.last_modified \u003d trait.created_at"},{"line_number":98,"context_line":"        req.response.cache_control \u003d \u0027no-cache\u0027"},{"line_number":99,"context_line":"    return req.response"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_3b3d6bdd","line":97,"updated":"2019-08-07 13:23:29.000000000","message":"If we reach the L91 then the \"trait\" variable points to the Trait object that was instantiated at L86 but it never persisted to the DB. So this trait.create_at will point to the incorrect creation time. \n\nI\u0027m not even sure if trait.created_at is set if trait.create() raised an exception. (in the unit test it is set to None but it can be due to mocking)","commit_id":"b2449e3f91a8d393d00abcb8195160b00772b232"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"aeaa8fe607aa23d0685d5dfb7356f9cdd7f779de","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    req.response.content_type \u003d None"},{"line_number":95,"context_line":"    req.response.location \u003d util.trait_url(req.environ, trait)"},{"line_number":96,"context_line":"    if want_version.matches((1, 15)):"},{"line_number":97,"context_line":"        req.response.last_modified \u003d trait.created_at"},{"line_number":98,"context_line":"        req.response.cache_control \u003d \u0027no-cache\u0027"},{"line_number":99,"context_line":"    return req.response"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_cd449a4a","line":97,"in_reply_to":"7faddb67_3b3d6bdd","updated":"2019-08-07 13:26:20.000000000","message":"nice catch, i\u0027ll see about fixing that, and making sure it has some kind of test","commit_id":"b2449e3f91a8d393d00abcb8195160b00772b232"}],"placement/tests/unit/handlers/test_trait.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f2db0421ffcdcd68d49dff42be218b44160ff0ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7faddb67_e410fd50","line":55,"updated":"2019-07-31 14:27:34.000000000","message":"I like this test, but what we need is a case where create raises TraitExists. That\u0027s the code path the gabbits don\u0027t exercise.","commit_id":"31d88703782f00271240a6a7197f6899c2698983"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3572fae20528de5c024f130384abc9a4fba04ecd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7faddb67_a4b9a515","line":55,"in_reply_to":"7faddb67_e410fd50","updated":"2019-07-31 14:36:25.000000000","message":"meh, fine, but also: http://p.anticdent.org/bT6q the death of life is on you","commit_id":"31d88703782f00271240a6a7197f6899c2698983"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"08eb8b08ba2a5d7d007b7393a34fa6f3c2fb2a02","unresolved":false,"context_lines":[{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":"\"\"\"Unit tests for code in the trait handler that gabbi cannot easily cover.\"\"\""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import microversion_parse"},{"line_number":17,"context_line":"import webob"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from placement import context"},{"line_number":20,"context_line":"from placement import exception"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_e9b5ea32","line":17,"range":{"start_line":14,"start_character":0,"end_line":17,"end_character":12},"updated":"2019-08-06 06:30:26.000000000","message":"nit:\n\nimport microversion_parse\nimport mock\nimport webob","commit_id":"baea7003603e6c957002d501fafe7f95896885d8"}]}