)]}'
{"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"9632187e67d7580887023752a50cae55bd881f74","unresolved":false,"context_lines":[{"line_number":4514,"context_line":"def action_finish(context, values):"},{"line_number":4515,"context_line":"    session \u003d get_session()"},{"line_number":4516,"context_line":"    with session.begin():"},{"line_number":4517,"context_line":"        action_ref \u003d model_query(context, models.InstanceAction,"},{"line_number":4518,"context_line":"                                 session\u003dsession).\\"},{"line_number":4519,"context_line":"                           filter_by(instance_uuid\u003dvalues[\u0027instance_uuid\u0027]).\\"},{"line_number":4520,"context_line":"                           filter_by(request_id\u003dvalues[\u0027request_id\u0027]).\\"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FyVg%3D","line":4517,"updated":"2013-01-04 23:56:28.000000000","message":"Here is a mistake..\nFor example if there is no InstanceActions with such criteria. \u003d\u003e action_ref is None \u003d\u003e action_ref.update(value) will raise Exception.","commit_id":"c1335f90d6d65c5daebf48ca9e367c3569cfbd7d"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"9632187e67d7580887023752a50cae55bd881f74","unresolved":false,"context_lines":[{"line_number":4555,"context_line":""},{"line_number":4556,"context_line":""},{"line_number":4557,"context_line":"def action_event_start(context, values):"},{"line_number":4558,"context_line":"    \"\"\"Start an event on an instance action\"\"\""},{"line_number":4559,"context_line":"    action \u003d _action_get_by_request_id(context, values[\u0027instance_uuid\u0027],"},{"line_number":4560,"context_line":"                                                values[\u0027request_id\u0027])"},{"line_number":4561,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FyU8%3D","line":4558,"updated":"2013-01-04 23:56:28.000000000","message":"For performance optimization this should be made in one transaction. So you should add in _action_get_by_request_id parameter session\u003dNone and use it in model_query.","commit_id":"c1335f90d6d65c5daebf48ca9e367c3569cfbd7d"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"9632187e67d7580887023752a50cae55bd881f74","unresolved":false,"context_lines":[{"line_number":4558,"context_line":"    \"\"\"Start an event on an instance action\"\"\""},{"line_number":4559,"context_line":"    action \u003d _action_get_by_request_id(context, values[\u0027instance_uuid\u0027],"},{"line_number":4560,"context_line":"                                                values[\u0027request_id\u0027])"},{"line_number":4561,"context_line":""},{"line_number":4562,"context_line":"    values[\u0027action_id\u0027] \u003d action[\u0027id\u0027]"},{"line_number":4563,"context_line":"    session \u003d get_session()"},{"line_number":4564,"context_line":"    with session.begin():"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FyVI%3D","line":4561,"updated":"2013-01-04 23:56:28.000000000","message":"Same problem here. If there is no action, we will got Exception.","commit_id":"c1335f90d6d65c5daebf48ca9e367c3569cfbd7d"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"9632187e67d7580887023752a50cae55bd881f74","unresolved":false,"context_lines":[{"line_number":4569,"context_line":""},{"line_number":4570,"context_line":""},{"line_number":4571,"context_line":"def action_event_finish(context, values):"},{"line_number":4572,"context_line":"    \"\"\"Finish an event on an instance action\"\"\""},{"line_number":4573,"context_line":"    action \u003d _action_get_by_request_id(context, values[\u0027instance_uuid\u0027],"},{"line_number":4574,"context_line":"                                                values[\u0027request_id\u0027])"},{"line_number":4575,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FyU4%3D","line":4572,"updated":"2013-01-04 23:56:28.000000000","message":"This method also should be made in one transaction.","commit_id":"c1335f90d6d65c5daebf48ca9e367c3569cfbd7d"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"9632187e67d7580887023752a50cae55bd881f74","unresolved":false,"context_lines":[{"line_number":4572,"context_line":"    \"\"\"Finish an event on an instance action\"\"\""},{"line_number":4573,"context_line":"    action \u003d _action_get_by_request_id(context, values[\u0027instance_uuid\u0027],"},{"line_number":4574,"context_line":"                                                values[\u0027request_id\u0027])"},{"line_number":4575,"context_line":""},{"line_number":4576,"context_line":"    session \u003d get_session()"},{"line_number":4577,"context_line":"    with session.begin():"},{"line_number":4578,"context_line":"        event_ref \u003d model_query(context, models.InstanceActionEvent,"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FyVQ%3D","line":4575,"updated":"2013-01-04 23:56:28.000000000","message":"Same problem here. If there is no action, we will got Exception.","commit_id":"c1335f90d6d65c5daebf48ca9e367c3569cfbd7d"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"8c15ee58825d23daea2d806489deda19a3264bad","unresolved":false,"context_lines":[{"line_number":4508,"context_line":""},{"line_number":4509,"context_line":"def action_start(context, values):"},{"line_number":4510,"context_line":"    session \u003d get_session()"},{"line_number":4511,"context_line":"    with session.begin():"},{"line_number":4512,"context_line":"        action_ref \u003d models.InstanceAction()"},{"line_number":4513,"context_line":"        action_ref.update(values)"},{"line_number":4514,"context_line":"        action_ref.save(session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fv74%3D","line":4511,"updated":"2013-01-07 19:29:55.000000000","message":"Sorry I didn\u0027t have enough time to look all carefully, but I\u0027m for clean code. I hope you\u0027re also for clean code.\n\nThere is no need here to create at this step session and transaction. We have only one db request and If you call NovaBase.save() method without session parameter it will create own session. Also if there is no transaction (our new situation) sqlalchemy will make a transaction.","commit_id":"660064948d0410b4b0046d15881cf468e2063521"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"8c15ee58825d23daea2d806489deda19a3264bad","unresolved":false,"context_lines":[{"line_number":4530,"context_line":"                                        instance_uuid\u003dvalues[\u0027instance_uuid\u0027])"},{"line_number":4531,"context_line":""},{"line_number":4532,"context_line":"        action_ref.update(values)"},{"line_number":4533,"context_line":"        action_ref.save(session\u003dsession)"},{"line_number":4534,"context_line":"    return action_ref"},{"line_number":4535,"context_line":""},{"line_number":4536,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fv9k%3D","line":4533,"updated":"2013-01-07 19:29:55.000000000","message":"If you are for clean code please remove action_ref.save(session\u003dsession). \n\nNovaBase.save() method does only this:\nsession.add(self) \nsession.flush()\nBut our object action_ref is already in session (so session.add(self) does nothing).\nAnd session.flush() will be called after session.commit() that is called in __exit__() hook. (so we don\u0027t need to call it explicitly)","commit_id":"660064948d0410b4b0046d15881cf468e2063521"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"8c15ee58825d23daea2d806489deda19a3264bad","unresolved":false,"context_lines":[{"line_number":4604,"context_line":"            raise exception.InstanceActionEventNotFound(action_id\u003daction[\u0027id\u0027],"},{"line_number":4605,"context_line":"                                                        event\u003dvalues[\u0027event\u0027])"},{"line_number":4606,"context_line":"        event_ref.update(values)"},{"line_number":4607,"context_line":"        event_ref.save(session\u003dsession)"},{"line_number":4608,"context_line":"    return event_ref"},{"line_number":4609,"context_line":""},{"line_number":4610,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fv84%3D","line":4607,"updated":"2013-01-07 19:29:55.000000000","message":"Same situation here.","commit_id":"660064948d0410b4b0046d15881cf468e2063521"}],"nova/db/sqlalchemy/migrate_repo/versions/146_add_instance_actions.py":[{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Note on the autoincrement flag: this is defaulted for primary key columns"},{"line_number":34,"context_line":"# of integral type, so is no longer set explicitly in such cases."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def upgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2Fujc%3D","line":33,"updated":"2013-01-08 15:49:12.000000000","message":"inline comments should look like: \n # NOTE(your_acc): first line....\n #                 second line","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"e32b3d38da34c5f3c07328cd67b6d25345c15862","unresolved":false,"context_lines":[{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# Note on the autoincrement flag: this is defaulted for primary key columns"},{"line_number":34,"context_line":"# of integral type, so is no longer set explicitly in such cases."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def upgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuXw%3D","line":33,"in_reply_to":"AAAAM3%2F%2Fujc%3D","updated":"2013-01-08 17:05:21.000000000","message":"I agree and I disagree.  I agree that this comment should be a NOTE(alaski): blah blah blah; however, I do not agree that this must be done with all comments.","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":88,"context_line":"def downgrade(migrate_engine):"},{"line_number":89,"context_line":"    meta \u003d MetaData()"},{"line_number":90,"context_line":"    meta.bind \u003d migrate_engine"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    instance_actions \u003d Table(\u0027instance_actions\u0027, meta, autoload\u003dTrue)"},{"line_number":93,"context_line":"    instance_actions.drop()"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FujA%3D","line":91,"updated":"2013-01-08 15:49:12.000000000","message":"I think that you have forgot try except statement here..","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"}],"nova/tests/test_db_api.py":[{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":550,"context_line":"                         \u0027start_time\u0027: start_time}"},{"line_number":551,"context_line":"        db.action_start(ctxt, action_values)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"        # Retrieve the action to ensure it was successfully added"},{"line_number":554,"context_line":"        actions \u003d db.actions_get(ctxt, uuid)"},{"line_number":555,"context_line":"        self.assertEqual(1, len(actions))"},{"line_number":556,"context_line":"        self.assertEqual(\u0027run_instance\u0027, actions[0][\u0027action\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuiY%3D","line":553,"updated":"2013-01-08 15:49:12.000000000","message":"Use standart format for inline comments (I described it in migration script )","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ff17dc5b48ddfd6a7bc3a7c2faee9d78f3abd98b","unresolved":false,"context_lines":[{"line_number":550,"context_line":"                         \u0027start_time\u0027: start_time}"},{"line_number":551,"context_line":"        db.action_start(ctxt, action_values)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"        # Retrieve the action to ensure it was successfully added"},{"line_number":554,"context_line":"        actions \u003d db.actions_get(ctxt, uuid)"},{"line_number":555,"context_line":"        self.assertEqual(1, len(actions))"},{"line_number":556,"context_line":"        self.assertEqual(\u0027run_instance\u0027, actions[0][\u0027action\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuR4%3D","line":553,"in_reply_to":"AAAAM3%2F%2FuXk%3D","updated":"2013-01-08 17:53:46.000000000","message":"My thinking is definitely in line with Kevin on this.  I added the comment just to highlight the testing method, i.e. testing that something was added by retrieving it versus using a mock or other method.  It helps me to quickly parse where to start looking if something goes wrong.  I\u0027m hesitant to start peppering my username all over the code so I would probably remove the comment before putting NOTE(alaski) in front of it.\n\nBut the more detailed comment in the migration script should have NOTE() on it.  I left it off though because it\u0027s a copy/paste from another script and not really my note.  I\u0027m not sure of the etiquette around that.","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"e32b3d38da34c5f3c07328cd67b6d25345c15862","unresolved":false,"context_lines":[{"line_number":550,"context_line":"                         \u0027start_time\u0027: start_time}"},{"line_number":551,"context_line":"        db.action_start(ctxt, action_values)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"        # Retrieve the action to ensure it was successfully added"},{"line_number":554,"context_line":"        actions \u003d db.actions_get(ctxt, uuid)"},{"line_number":555,"context_line":"        self.assertEqual(1, len(actions))"},{"line_number":556,"context_line":"        self.assertEqual(\u0027run_instance\u0027, actions[0][\u0027action\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuXk%3D","line":553,"in_reply_to":"AAAAM3%2F%2FuiY%3D","updated":"2013-01-08 17:05:21.000000000","message":"I disagree.  This is a simple inline comment that augments the reader\u0027s understanding of what this segment of code is doing.  If it were a detailed note written to give a coder looking to modify the code some things they should keep in mind, then it should be a NOTE(); if it were a to-do item, then it should be a TODO().  I want to encourage coders to liberally comment their code; being too legalistic about the block comment format is going to have the opposite effect.\n\nAs a final note, it is worth pointing out that there is no guidance regarding general comments in HACKING.rst; the only mention is in conjunction with TODOs.","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":575,"context_line":"                                \u0027finish_time\u0027: finish_time}"},{"line_number":576,"context_line":"        db.action_finish(ctxt, action_finish_values)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        # Retrieve the action to ensure it was successfully added"},{"line_number":579,"context_line":"        actions \u003d db.actions_get(ctxt, uuid)"},{"line_number":580,"context_line":"        self.assertEqual(1, len(actions))"},{"line_number":581,"context_line":"        self.assertEqual(\u0027run_instance\u0027, actions[0][\u0027action\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuiU%3D","line":578,"updated":"2013-01-08 15:49:12.000000000","message":"same here","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":606,"context_line":"        db.action_start(ctxt2, action_values)"},{"line_number":607,"context_line":"        db.action_start(ctxt2, action_values)"},{"line_number":608,"context_line":""},{"line_number":609,"context_line":"        # Retrieve the action to ensure it was successfully added"},{"line_number":610,"context_line":"        actions \u003d db.actions_get(ctxt1, uuid1)"},{"line_number":611,"context_line":"        self.assertEqual(2, len(actions))"},{"line_number":612,"context_line":"        self.assertEqual(\u0027resize\u0027, actions[0][\u0027action\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuiQ%3D","line":609,"updated":"2013-01-08 15:49:12.000000000","message":"same","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":661,"context_line":"                        \u0027start_time\u0027: start_time}"},{"line_number":662,"context_line":"        db.action_event_start(ctxt, event_values)"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        # Retrieve the event to ensure it was successfully added"},{"line_number":665,"context_line":"        events \u003d db.action_events_get(ctxt, action[\u0027id\u0027])"},{"line_number":666,"context_line":"        self.assertEqual(1, len(events))"},{"line_number":667,"context_line":"        self.assertEqual(\u0027schedule\u0027, events[0][\u0027event\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuiM%3D","line":664,"updated":"2013-01-08 15:49:12.000000000","message":"same","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":693,"context_line":"                                \u0027finish_time\u0027: finish_time}"},{"line_number":694,"context_line":"        db.action_event_finish(ctxt, event_finish_values)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        # Retrieve the event to ensure it was successfully added"},{"line_number":697,"context_line":"        events \u003d db.action_events_get(ctxt, action[\u0027id\u0027])"},{"line_number":698,"context_line":"        self.assertEqual(1, len(events))"},{"line_number":699,"context_line":"        self.assertEqual(\u0027schedule\u0027, events[0][\u0027event\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuiI%3D","line":696,"updated":"2013-01-08 15:49:12.000000000","message":"same","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aca7b116ea1a2c5ea1a5dbc7a9588d1141f74b23","unresolved":false,"context_lines":[{"line_number":734,"context_line":"                        \u0027start_time\u0027: timeutils.utcnow()}"},{"line_number":735,"context_line":"        db.action_event_start(ctxt2, event_values)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"        # Retrieve the event to ensure it was successfully added"},{"line_number":738,"context_line":"        event \u003d db.action_event_get_by_id(ctxt1, added_action[\u0027id\u0027],"},{"line_number":739,"context_line":"                                                     added_event[\u0027id\u0027])"},{"line_number":740,"context_line":"        self.assertEqual(\u0027schedule\u0027, event[\u0027event\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FuiE%3D","line":737,"updated":"2013-01-08 15:49:12.000000000","message":"same","commit_id":"5cd60a3536da35ac02302b88e595cd246d415902"}]}
