)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c2ed3a527052622c2dc7a1dce3b3ef7d45bd833a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Pushkar Umaranikar \u003cpushkar.umaranikar@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-02-06 21:26:00 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Placement api: Add json_error_formatter to defaults"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Any time placement API (v1.0) code wants to raise a webob.exc of some kind it"},{"line_number":10,"context_line":"needs to add a json_formatter attribute to the call. This is easy to forget."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1a430d35_56687887","line":7,"updated":"2017-02-07 13:49:44.000000000","message":"Nit: This line isn\u0027t a great summary of this patch any more.\n\nMaybe something like: \"Moving json_error_formatter in allocations\"?","commit_id":"3c680cd9b613efed95d158375ad02b19bfddc17d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"45f2e4fd3d842d37d4c3061dbaa96cda8b43cc65","unresolved":false,"context_lines":[{"line_number":14,"context_line":"wrapper as decorater instead of webob.dec.wsgify."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In this change, decorator updated for allocations related actions."},{"line_number":17,"context_line":"Change in other resource classes can be addressed in subsequent patches."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I3b81c5bd00a013f1659b9e6e80c71b373d965862"},{"line_number":20,"context_line":"Partial-Bug: #1635182"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"1a430d35_25964b93","line":17,"updated":"2017-02-07 16:43:54.000000000","message":"Because of the use of the term \"resource classes\" elsewhere in the placement API I\u0027d suggest \"resource handlers\" here, but only if you need to make another version for some other reason.","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12aa39f373457344729d6cddb5e9f20fb5e5430d","unresolved":false,"context_lines":[{"line_number":14,"context_line":"wrapper as decorater instead of webob.dec.wsgify."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In this change, decorator updated for allocations related actions."},{"line_number":17,"context_line":"Change in other resource classes can be addressed in subsequent patches."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I3b81c5bd00a013f1659b9e6e80c71b373d965862"},{"line_number":20,"context_line":"Partial-Bug: #1635182"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"1a430d35_40c34538","line":17,"in_reply_to":"1a430d35_25964b93","updated":"2017-02-07 17:02:03.000000000","message":"+1","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"},{"author":{"_account_id":18335,"name":"Pushkar Umaranikar","email":"pushkar.umaranikar@intel.com","username":"pushkarumaranikar"},"change_message_id":"e608a8cac72b4028ca922a7868d7aaf2c12c7f78","unresolved":false,"context_lines":[{"line_number":14,"context_line":"wrapper as decorater instead of webob.dec.wsgify."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In this change, decorator updated for allocations related actions."},{"line_number":17,"context_line":"Change in other resource classes can be addressed in subsequent patches."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I3b81c5bd00a013f1659b9e6e80c71b373d965862"},{"line_number":20,"context_line":"Partial-Bug: #1635182"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"1a430d35_579bc865","line":17,"in_reply_to":"1a430d35_40c34538","updated":"2017-02-08 21:49:20.000000000","message":"Done","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"}],"nova/api/openstack/placement/handlers/allocation.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b964de54ed1ff2dd0716c4c6f9886de691bdd6ad","unresolved":false,"context_lines":[{"line_number":20,"context_line":"import webob"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova.api.openstack.placement import util"},{"line_number":23,"context_line":"from nova.api.openstack.placement import wsgi_wrapper "},{"line_number":24,"context_line":"from nova import exception"},{"line_number":25,"context_line":"from nova.i18n import _, _LE"},{"line_number":26,"context_line":"from nova import objects"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_732f6ff6","line":23,"updated":"2017-02-06 16:48:05.000000000","message":"nit whitespace","commit_id":"3eb12133529c6cb5b093ab5dccbe50b15bebee7f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c2ed3a527052622c2dc7a1dce3b3ef7d45bd833a","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                             resource_provider\u003dresource_provider)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"@webob.dec.wsgify"},{"line_number":161,"context_line":"@util.check_accept(\u0027application/json\u0027)"},{"line_number":162,"context_line":"def list_for_consumer(req):"},{"line_number":163,"context_line":"    \"\"\"List allocations associated with a consumer.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_f6c6c498","side":"PARENT","line":160,"range":{"start_line":160,"start_character":0,"end_line":160,"end_character":17},"updated":"2017-02-07 13:49:44.000000000","message":"Its tempting to put a pep8 test in to make sure we never call this method.\n\nBut thats totally for the last one in the series.","commit_id":"1129a9ff474529be49dbf5e55ee392cc5ca4cd84"}],"nova/api/openstack/placement/wsgi_wrapper.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b964de54ed1ff2dd0716c4c6f9886de691bdd6ad","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from nova.api.openstack.placement import util"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class PlacementWsgify(wsgify):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def call_func(self, req, *args, **kwargs):"},{"line_number":22,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_13fe2b37","line":19,"updated":"2017-02-06 16:48:05.000000000","message":"Yeah, this is pretty much what I was thinking. Does it work okay? :)\n\nI think with a few docstrings and related cleanups this could work well.\n\nI\u0027m not sure about the name though. It is annoyingly long, but I may only think that because of the shortness of what is replacing.","commit_id":"3eb12133529c6cb5b093ab5dccbe50b15bebee7f"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b964de54ed1ff2dd0716c4c6f9886de691bdd6ad","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def call_func(self, req, *args, **kwargs):"},{"line_number":22,"context_line":"        try:"},{"line_number":23,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs) "},{"line_number":24,"context_line":"        except webob.exc.HTTPException as exc:"},{"line_number":25,"context_line":"            exc.json_formatter \u003d util.json_error_formatter"},{"line_number":26,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_b310f7a9","line":23,"updated":"2017-02-06 16:48:05.000000000","message":"more nit: whitespace","commit_id":"3eb12133529c6cb5b093ab5dccbe50b15bebee7f"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"291c9a0029eba5c5b8ca84104a3204309a0ba5dd","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        try:"},{"line_number":32,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"},{"line_number":33,"context_line":"        except webob.exc.HTTPException as exc:"},{"line_number":34,"context_line":"            LOG.error(_(\"Error occured in Placement API. Setting custom json\""},{"line_number":35,"context_line":"                      \"formatter to write error response.\"))"},{"line_number":36,"context_line":"            exc.json_formatter \u003d util.json_error_formatter"},{"line_number":37,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_65deb740","line":34,"range":{"start_line":34,"start_character":72,"end_line":34,"end_character":76},"updated":"2017-02-06 23:59:23.000000000","message":"nit: add a space after json.","commit_id":"3c680cd9b613efed95d158375ad02b19bfddc17d"},{"author":{"_account_id":18335,"name":"Pushkar Umaranikar","email":"pushkar.umaranikar@intel.com","username":"pushkarumaranikar"},"change_message_id":"cbc3e1fe0e748d2cdbd712326ea688771fe9b5c1","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        try:"},{"line_number":32,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"},{"line_number":33,"context_line":"        except webob.exc.HTTPException as exc:"},{"line_number":34,"context_line":"            LOG.error(_(\"Error occured in Placement API. Setting custom json\""},{"line_number":35,"context_line":"                      \"formatter to write error response.\"))"},{"line_number":36,"context_line":"            exc.json_formatter \u003d util.json_error_formatter"},{"line_number":37,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_f37976c9","line":34,"range":{"start_line":34,"start_character":72,"end_line":34,"end_character":76},"in_reply_to":"1a430d35_65deb740","updated":"2017-02-07 13:31:47.000000000","message":"I will address this in following subsequent patch.","commit_id":"3c680cd9b613efed95d158375ad02b19bfddc17d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c2ed3a527052622c2dc7a1dce3b3ef7d45bd833a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        try:"},{"line_number":32,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"},{"line_number":33,"context_line":"        except webob.exc.HTTPException as exc:"},{"line_number":34,"context_line":"            LOG.error(_(\"Error occured in Placement API. Setting custom json\""},{"line_number":35,"context_line":"                      \"formatter to write error response.\"))"},{"line_number":36,"context_line":"            exc.json_formatter \u003d util.json_error_formatter"},{"line_number":37,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a430d35_f6cc046c","line":34,"range":{"start_line":34,"start_character":72,"end_line":34,"end_character":76},"in_reply_to":"1a430d35_f37976c9","updated":"2017-02-07 13:49:44.000000000","message":"I don\u0027t think we want LOG.error here, that adds ERROR logs in addition to the existing logs that are raised.\n\nAs most of these errors has nothing to do with the operator, and indeed many of them are expected, I am not sure we want to add an ERROR log. Please see:\nhttps://specs.openstack.org/openstack/openstack-specs/specs/log-guidelines.html\n\nThere is also no context on the error in this log message, and it should be using _LE handler for the log so it gets categorised correctly.\n\nNow LOG.debug probably does make sense here, doing something like:\n\n    LOG.debug(\"Placement API returning an error response: %s\", exc)","commit_id":"3c680cd9b613efed95d158375ad02b19bfddc17d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"45f2e4fd3d842d37d4c3061dbaa96cda8b43cc65","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def call_func(self, req, *args, **kwargs):"},{"line_number":27,"context_line":"        \"\"\"Serve as wrapper for call_func from webob to add custom json"},{"line_number":28,"context_line":"           error formatter."},{"line_number":29,"context_line":"        \"\"\""},{"line_number":30,"context_line":"        try:"},{"line_number":31,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a430d35_65ba130c","line":28,"updated":"2017-02-07 16:43:54.000000000","message":"Is this formatting aligned with how we normally do things?","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12aa39f373457344729d6cddb5e9f20fb5e5430d","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def call_func(self, req, *args, **kwargs):"},{"line_number":27,"context_line":"        \"\"\"Serve as wrapper for call_func from webob to add custom json"},{"line_number":28,"context_line":"           error formatter."},{"line_number":29,"context_line":"        \"\"\""},{"line_number":30,"context_line":"        try:"},{"line_number":31,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a430d35_e086f1d9","line":28,"in_reply_to":"1a430d35_65ba130c","updated":"2017-02-07 17:02:03.000000000","message":"Not really I guess, first line should be a one liner really, matching the usual pep guidelines:\nhttp://docs.openstack.org/developer/hacking/#docstrings\n\nBut I wouldn\u0027t block on that myself, but maybe I should.","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"},{"author":{"_account_id":18335,"name":"Pushkar Umaranikar","email":"pushkar.umaranikar@intel.com","username":"pushkarumaranikar"},"change_message_id":"e608a8cac72b4028ca922a7868d7aaf2c12c7f78","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def call_func(self, req, *args, **kwargs):"},{"line_number":27,"context_line":"        \"\"\"Serve as wrapper for call_func from webob to add custom json"},{"line_number":28,"context_line":"           error formatter."},{"line_number":29,"context_line":"        \"\"\""},{"line_number":30,"context_line":"        try:"},{"line_number":31,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a430d35_1795c031","line":28,"in_reply_to":"1a430d35_e086f1d9","updated":"2017-02-08 21:49:20.000000000","message":"Done","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"45f2e4fd3d842d37d4c3061dbaa96cda8b43cc65","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        try:"},{"line_number":31,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"},{"line_number":32,"context_line":"        except webob.exc.HTTPException as exc:"},{"line_number":33,"context_line":"            LOG.debug(\"Placement API returning an error response: %s\", exc)"},{"line_number":34,"context_line":"            exc.json_formatter \u003d util.json_error_formatter"},{"line_number":35,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a430d35_e5b363e7","line":33,"updated":"2017-02-07 16:43:54.000000000","message":"I\u0027m undecided if we should do this logging here or not. I\u0027d rather we instead got in the habit of making sure that we logged where we raise (as needed) and if we do that, then also logging here will result in a lot of logging.\n\nI suspect John and Sean can provided more well informed opinions on that.","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12aa39f373457344729d6cddb5e9f20fb5e5430d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"        try:"},{"line_number":31,"context_line":"            super(PlacementWsgify, self).call_func(req, *args, **kwargs)"},{"line_number":32,"context_line":"        except webob.exc.HTTPException as exc:"},{"line_number":33,"context_line":"            LOG.debug(\"Placement API returning an error response: %s\", exc)"},{"line_number":34,"context_line":"            exc.json_formatter \u003d util.json_error_formatter"},{"line_number":35,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a430d35_e001916b","line":33,"in_reply_to":"1a430d35_e5b363e7","updated":"2017-02-07 17:02:03.000000000","message":"Well if its something that needs operator attention, it would be good to do that elsewhere, I am OK with a catch all debug, I am also OK with no logging here.\n\nWhat changed my mind was debugging 409 errors recently, having this log here would have helped me. Turns out we log it on the Nova side when its an issue, so it wasn\u0027t needed, but it would be handy for debug.","commit_id":"c87a8ac91b6401babef96824981cbfe7aa22db8e"}]}
