)]}'
{"climate/db/migration/alembic_migrations/versions/23d6240b51b2_add_status_to_leases.py":[{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"9291c8527c3db4cecbdcb8979bfb592a9b058889","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.add_column(\u0027leases\u0027, sa.Column("},{"line_number":34,"context_line":"        \u0027action\u0027, sa.String(length\u003d255), nullable\u003dTrue))"},{"line_number":35,"context_line":"    op.add_column(\u0027leases\u0027, sa.Column("}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_9b5ce427","line":32,"updated":"2014-05-07 17:48:25.000000000","message":"Should we use default values for the existing rows in DB? What is your plan for the existing leases without state information?","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"8bb8b95a3dd3d71ed366f0cd104e7aba4fed9273","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.add_column(\u0027leases\u0027, sa.Column("},{"line_number":34,"context_line":"        \u0027action\u0027, sa.String(length\u003d255), nullable\u003dTrue))"},{"line_number":35,"context_line":"    op.add_column(\u0027leases\u0027, sa.Column("}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_ebc31988","line":32,"in_reply_to":"7adec928_456eca02","updated":"2014-05-22 15:19:41.000000000","message":"Ok","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f2bffc3007a4bad6401957224f82fe19470af6f7","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.add_column(\u0027leases\u0027, sa.Column("},{"line_number":34,"context_line":"        \u0027action\u0027, sa.String(length\u003d255), nullable\u003dTrue))"},{"line_number":35,"context_line":"    op.add_column(\u0027leases\u0027, sa.Column("}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_456eca02","line":32,"in_reply_to":"7adec928_9b5ce427","updated":"2014-05-22 15:13:37.000000000","message":"That\u0027s an excellent question. As you can see, leases statuses can be set to None, that\u0027s only when updating the status that it will check.\nSo, when upgrading Climate while a lease is still created, it will be set to None until an action is performed.","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"}],"climate/states.py":[{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"4fee6b3f3d50156f8152a12d7ef6a0a8e4375e60","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                \u0027status_reason\u0027: self.status_reason}"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def update(self, action, status, status_reason\u003dNone):"},{"line_number":56,"context_line":"        if action not in self.ACTIONS or status not in self.STATUSES:"},{"line_number":57,"context_line":"            LOG.warning(_LW("},{"line_number":58,"context_line":"                \"Unable to update ID ({0}) state with {1}:{2}\".format("},{"line_number":59,"context_line":"                id, action, status)))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a1371a2_070a38d2","line":56,"updated":"2014-04-28 20:51:20.000000000","message":"I think it could be a good idea to check if action or status are None. If someone inherits from this class and does not fill action/status properly during init, a set of warning with None action/status would be logged, and that could be unnoticed by the developer.\nBTW, with the actual LeaseState implementation, a warning will be logged if it is created with default values and a non existing lease.","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08f138b26a6b863d406749491d40a995b4bb060c","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                \u0027status_reason\u0027: self.status_reason}"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def update(self, action, status, status_reason\u003dNone):"},{"line_number":56,"context_line":"        if action not in self.ACTIONS or status not in self.STATUSES:"},{"line_number":57,"context_line":"            LOG.warning(_LW("},{"line_number":58,"context_line":"                \"Unable to update ID ({0}) state with {1}:{2}\".format("},{"line_number":59,"context_line":"                id, action, status)))"}],"source_content_type":"text/x-python","patch_set":2,"id":"bad4c104_96991171","line":56,"in_reply_to":"3a1371a2_070a38d2","updated":"2014-04-29 14:30:13.000000000","message":"You\u0027re right, I\u0027m modifying it.","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"4fee6b3f3d50156f8152a12d7ef6a0a8e4375e60","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        except db_exc.ClimateDBException:"},{"line_number":95,"context_line":"            # Lease can be not yet in DB, we must first write it"},{"line_number":96,"context_line":"            raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":97,"context_line":"        if self.current() \u003d\u003d {\u0027action\u0027: lease.get(\u0027action\u0027),"},{"line_number":98,"context_line":"                              \u0027status\u0027: lease.get(\u0027status\u0027),"},{"line_number":99,"context_line":"                              \u0027status_reason\u0027: lease.get(\u0027status_reason\u0027)}:"},{"line_number":100,"context_line":"            return self.current()"}],"source_content_type":"text/x-python","patch_set":2,"id":"facab960_c2abe2d4","line":97,"updated":"2014-04-28 20:51:20.000000000","message":"I can\u0027t get the purpose of this piece of code. Could you explain me?","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08f138b26a6b863d406749491d40a995b4bb060c","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        except db_exc.ClimateDBException:"},{"line_number":95,"context_line":"            # Lease can be not yet in DB, we must first write it"},{"line_number":96,"context_line":"            raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":97,"context_line":"        if self.current() \u003d\u003d {\u0027action\u0027: lease.get(\u0027action\u0027),"},{"line_number":98,"context_line":"                              \u0027status\u0027: lease.get(\u0027status\u0027),"},{"line_number":99,"context_line":"                              \u0027status_reason\u0027: lease.get(\u0027status_reason\u0027)}:"},{"line_number":100,"context_line":"            return self.current()"}],"source_content_type":"text/x-python","patch_set":2,"id":"bad4c104_b6ec55c9","line":97,"in_reply_to":"facab960_c2abe2d4","updated":"2014-04-29 14:30:13.000000000","message":"Here, we need to make sure the update correctly happened in DB so that the lease values are identical to the object.\n\nBut, indeed, after thinking about it, it\u0027s covered by unittests, no need to have it here.","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"980340eafb1bddc3d921931bf4e7dd14e557e3e1","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2014 Red Hat."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":4,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_1c0364b9","line":1,"updated":"2014-04-30 10:45:45.000000000","message":"hmm .. didn\u0027t know about RedHat contributributions, welcome :)","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":9225,"name":"Cristian A Sanchez","email":"cabsanchez@gmail.com","username":"casanch1"},"change_message_id":"c7b0f05a5f102ab69d3119e06da616e96c39c8dc","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                \u0027status_reason\u0027: self.status_reason}"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def update(self, action, status, status_reason\u003dNone):"},{"line_number":57,"context_line":"        if action not in self.ACTIONS or status not in self.STATUSES:"},{"line_number":58,"context_line":"            LOG.warning(_LW("},{"line_number":59,"context_line":"                \"Unable to update ID ({0}) state with {1}:{2}\".format("},{"line_number":60,"context_line":"                id, action, status)))"}],"source_content_type":"text/x-python","patch_set":3,"id":"bad4c104_01a0568f","line":57,"updated":"2014-04-29 16:42:12.000000000","message":"Why don\u0027t you throw an exception here? If action is not in self.ACTIONS, then something really weird happened.","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":3012,"name":"Dina Belova","email":"dbelova@mirantis.com","username":"dbelova"},"change_message_id":"339b309f6efa27f65174d07da2bb2ebbdfc9fd61","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                \u0027status_reason\u0027: self.status_reason}"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def update(self, action, status, status_reason\u003dNone):"},{"line_number":57,"context_line":"        if action not in self.ACTIONS or status not in self.STATUSES:"},{"line_number":58,"context_line":"            LOG.warning(_LW("},{"line_number":59,"context_line":"                \"Unable to update ID ({0}) state with {1}:{2}\".format("},{"line_number":60,"context_line":"                id, action, status)))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_f3a42159","line":57,"in_reply_to":"9ad9bd40_fc916053","updated":"2014-04-30 17:43:23.000000000","message":"+1","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"980340eafb1bddc3d921931bf4e7dd14e557e3e1","unresolved":false,"context_lines":[{"line_number":54,"context_line":"                \u0027status_reason\u0027: self.status_reason}"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def update(self, action, status, status_reason\u003dNone):"},{"line_number":57,"context_line":"        if action not in self.ACTIONS or status not in self.STATUSES:"},{"line_number":58,"context_line":"            LOG.warning(_LW("},{"line_number":59,"context_line":"                \"Unable to update ID ({0}) state with {1}:{2}\".format("},{"line_number":60,"context_line":"                id, action, status)))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_fc916053","line":57,"in_reply_to":"bad4c104_01a0568f","updated":"2014-04-30 10:45:45.000000000","message":"agree with Cristian","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"980340eafb1bddc3d921931bf4e7dd14e557e3e1","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            LOG.warning(_LW("},{"line_number":59,"context_line":"                \"Unable to update ID ({0}) state with {1}:{2}\".format("},{"line_number":60,"context_line":"                id, action, status)))"},{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            self.action \u003d action"},{"line_number":63,"context_line":"            self.status \u003d status"},{"line_number":64,"context_line":"            self.status_reason \u003d status_reason"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_7c947037","line":61,"updated":"2014-04-30 10:45:45.000000000","message":"if an exception is raised above, no more need this \"else\"","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":9225,"name":"Cristian A Sanchez","email":"cabsanchez@gmail.com","username":"casanch1"},"change_message_id":"c7b0f05a5f102ab69d3119e06da616e96c39c8dc","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            self.status \u003d status"},{"line_number":64,"context_line":"            self.status_reason \u003d status_reason"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        if self.autosave is True:"},{"line_number":67,"context_line":"            self.save()"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"bad4c104_e12452c8","line":66,"updated":"2014-04-29 16:42:12.000000000","message":"I think this should be moved to the else: block of the previous IF statement.","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7494,"name":"Guangya Liu","email":"gyliu513@gmail.com","username":"JayLau"},"change_message_id":"37f1c828446eb4f1af6f290a551644d9c546b5a0","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            if lease is not None:"},{"line_number":100,"context_line":"                return self.current()"},{"line_number":101,"context_line":"            else:"},{"line_number":102,"context_line":"                raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"# NOTE(sbauza): For convenient purpose"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_f60c94ef","line":101,"updated":"2014-05-06 09:17:22.000000000","message":"nit: seems this else is not needed.\n\nif lease is not None:\n    return self.current()\nraise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"be9e0047a5993055986fe0a9b8bc6e27550adb99","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            if lease is not None:"},{"line_number":100,"context_line":"                return self.current()"},{"line_number":101,"context_line":"            else:"},{"line_number":102,"context_line":"                raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"# NOTE(sbauza): For convenient purpose"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_232ebb92","line":101,"in_reply_to":"9ad9bd40_f60c94ef","updated":"2014-05-06 12:41:02.000000000","message":"+1","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"980340eafb1bddc3d921931bf4e7dd14e557e3e1","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"# NOTE(sbauza): For convenient purpose"},{"line_number":105,"context_line":"lease \u003d LeaseState"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_5c040c7e","line":105,"updated":"2014-04-30 10:45:45.000000000","message":"don\u0027t sure it\u0027s more convenient, you don\u0027t  even use it in tests climate/tests/test_states.py :)","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"9291c8527c3db4cecbdcb8979bfb592a9b058889","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __init__(self, id, autosave\u003dTrue,"},{"line_number":77,"context_line":"                 action\u003dNone, status\u003dNone, status_reason\u003dNone):"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if action is None or status is None:"},{"line_number":80,"context_line":"            # NOTE(sbauza): The lease can be not yet in DB, so lease_get can"},{"line_number":81,"context_line":"            #               return None"},{"line_number":82,"context_line":"            lease \u003d db_api.lease_get(id) or {}"}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_48dd3014","line":79,"updated":"2014-05-07 17:48:25.000000000","message":"If only one of this parameters are not None, the value of that parameter will be updated with DB value, instead of keeping the actual value.\nI can\u0027t foresee if this could be an issue, but if you want to update only the status during the initialization of the object with action\u003dNone the status wont be updated. e.g.\n  LeaseState(\u00271\u0027, status\u003dFAILED)\n\nSorry, I missed this the last time.","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f2bffc3007a4bad6401957224f82fe19470af6f7","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __init__(self, id, autosave\u003dTrue,"},{"line_number":77,"context_line":"                 action\u003dNone, status\u003dNone, status_reason\u003dNone):"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if action is None or status is None:"},{"line_number":80,"context_line":"            # NOTE(sbauza): The lease can be not yet in DB, so lease_get can"},{"line_number":81,"context_line":"            #               return None"},{"line_number":82,"context_line":"            lease \u003d db_api.lease_get(id) or {}"}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_b7b98530","line":79,"in_reply_to":"7adec928_48dd3014","updated":"2014-05-22 15:13:37.000000000","message":"IMHO  it doesn\u0027t make sense to only provide a status without an action. That\u0027s a design choice I made, maybe we can change it in a next patch, don\u0027t you think ?","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"8bb8b95a3dd3d71ed366f0cd104e7aba4fed9273","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __init__(self, id, autosave\u003dTrue,"},{"line_number":77,"context_line":"                 action\u003dNone, status\u003dNone, status_reason\u003dNone):"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if action is None or status is None:"},{"line_number":80,"context_line":"            # NOTE(sbauza): The lease can be not yet in DB, so lease_get can"},{"line_number":81,"context_line":"            #               return None"},{"line_number":82,"context_line":"            lease \u003d db_api.lease_get(id) or {}"}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_cbf65df9","line":79,"in_reply_to":"7adec928_b7b98530","updated":"2014-05-22 15:19:41.000000000","message":"I\u0027m OK, if we need that change we can change it in a next patch.","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"9291c8527c3db4cecbdcb8979bfb592a9b058889","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        else:"},{"line_number":96,"context_line":"            if lease is not None:"},{"line_number":97,"context_line":"                return self.current()"},{"line_number":98,"context_line":"            raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"# NOTE(sbauza): For convenient purpose"},{"line_number":101,"context_line":"lease \u003d LeaseState"}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_08fc083e","line":98,"updated":"2014-05-07 17:48:25.000000000","message":"In which situation this exception could be raised?","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f2bffc3007a4bad6401957224f82fe19470af6f7","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        else:"},{"line_number":96,"context_line":"            if lease is not None:"},{"line_number":97,"context_line":"                return self.current()"},{"line_number":98,"context_line":"            raise mgr_exc.InvalidState(id\u003dself.id, state\u003dself.current())"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"# NOTE(sbauza): For convenient purpose"},{"line_number":101,"context_line":"lease \u003d LeaseState"}],"source_content_type":"text/x-python","patch_set":6,"id":"7adec928_d78d317d","line":98,"in_reply_to":"7adec928_08fc083e","updated":"2014-05-22 15:13:37.000000000","message":"agree, this is not necessary.","commit_id":"a74c842f4d7df37939dc1adccc0be786bee711df"}],"climate/tests/test_states.py":[{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"4fee6b3f3d50156f8152a12d7ef6a0a8e4375e60","unresolved":false,"context_lines":[{"line_number":73,"context_line":"                    \u0027status_reason\u0027: None}"},{"line_number":74,"context_line":"        self.assertEqual(expected, self.leaseState.current())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_current_state(self):"},{"line_number":77,"context_line":"        self.leaseState \u003d states.LeaseState(id\u003d1)"},{"line_number":78,"context_line":"        expected \u003d {\u0027action\u0027: None,"},{"line_number":79,"context_line":"                    \u0027status\u0027: None,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a1371a2_ca5147da","line":76,"updated":"2014-04-28 20:51:20.000000000","message":"Seems that the purpose of this test is to check that current() returns all the attributes in a dictionary. The thing is this is also covered by test_state_init. Maybe this method could be removed.","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08f138b26a6b863d406749491d40a995b4bb060c","unresolved":false,"context_lines":[{"line_number":73,"context_line":"                    \u0027status_reason\u0027: None}"},{"line_number":74,"context_line":"        self.assertEqual(expected, self.leaseState.current())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_current_state(self):"},{"line_number":77,"context_line":"        self.leaseState \u003d states.LeaseState(id\u003d1)"},{"line_number":78,"context_line":"        expected \u003d {\u0027action\u0027: None,"},{"line_number":79,"context_line":"                    \u0027status\u0027: None,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bad4c104_b6037572","line":76,"in_reply_to":"3a1371a2_ca5147da","updated":"2014-04-29 14:30:13.000000000","message":"Done","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"4fee6b3f3d50156f8152a12d7ef6a0a8e4375e60","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        self.assertEqual(0, self.lease_update.call_count)"},{"line_number":113,"context_line":"        self.assertEqual(expected, self.leaseState.current())"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def test_update_state_with_incorrect_action(self):"},{"line_number":116,"context_line":"        self.leaseState \u003d states.LeaseState(id\u003d1)"},{"line_number":117,"context_line":"        expected \u003d {\u0027action\u0027: None,"},{"line_number":118,"context_line":"                    \u0027status\u0027: None,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a1371a2_8acaaf6c","line":115,"updated":"2014-04-28 20:51:20.000000000","message":"Rename this to test_update_state_with_incorrect_action_status() or something that includes the word status.","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08f138b26a6b863d406749491d40a995b4bb060c","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        self.assertEqual(0, self.lease_update.call_count)"},{"line_number":113,"context_line":"        self.assertEqual(expected, self.leaseState.current())"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def test_update_state_with_incorrect_action(self):"},{"line_number":116,"context_line":"        self.leaseState \u003d states.LeaseState(id\u003d1)"},{"line_number":117,"context_line":"        expected \u003d {\u0027action\u0027: None,"},{"line_number":118,"context_line":"                    \u0027status\u0027: None,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bad4c104_96359148","line":115,"in_reply_to":"3a1371a2_8acaaf6c","updated":"2014-04-29 14:30:13.000000000","message":"Done","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"4fee6b3f3d50156f8152a12d7ef6a0a8e4375e60","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        self.leaseState.update(action\u003dstates.LeaseState.CREATE,"},{"line_number":130,"context_line":"                               status\u003dstates.LeaseState.IN_PROGRESS,"},{"line_number":131,"context_line":"                               status_reason\u003d\"Creating Lease...\")"},{"line_number":132,"context_line":"        self.assertEqual(None, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":133,"context_line":"        self.leaseState.save()"},{"line_number":134,"context_line":"        self.assertEqual(\u0027CREATE\u0027, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a1371a2_cad787ec","line":132,"updated":"2014-04-28 20:51:20.000000000","message":"I think that is better to assert this using call_count(). Otherwise, the assertion depends on the mock implementation (fake_lease_update method)","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08f138b26a6b863d406749491d40a995b4bb060c","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        self.leaseState.update(action\u003dstates.LeaseState.CREATE,"},{"line_number":130,"context_line":"                               status\u003dstates.LeaseState.IN_PROGRESS,"},{"line_number":131,"context_line":"                               status_reason\u003d\"Creating Lease...\")"},{"line_number":132,"context_line":"        self.assertEqual(None, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":133,"context_line":"        self.leaseState.save()"},{"line_number":134,"context_line":"        self.assertEqual(\u0027CREATE\u0027, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bad4c104_b63a9534","line":132,"in_reply_to":"3a1371a2_cad787ec","updated":"2014-04-29 14:30:13.000000000","message":"Done","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":9331,"name":"Pablo Andres Fuente","email":"pablo.andres.fuente@gmail.com","username":"pafuent"},"change_message_id":"4fee6b3f3d50156f8152a12d7ef6a0a8e4375e60","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                               status_reason\u003d\"Creating Lease...\")"},{"line_number":132,"context_line":"        self.assertEqual(None, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":133,"context_line":"        self.leaseState.save()"},{"line_number":134,"context_line":"        self.assertEqual(\u0027CREATE\u0027, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_save_state_with_nonexisting_lease(self):"},{"line_number":137,"context_line":"        def fake_lease_update_raise(id, values):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a1371a2_2a045b7d","line":134,"updated":"2014-04-28 20:51:20.000000000","message":"I think that is better to assert this using assert_called_once_with(). Otherwise, the assertion depends on the mock implementation (fake_lease_update method)","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08f138b26a6b863d406749491d40a995b4bb060c","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                               status_reason\u003d\"Creating Lease...\")"},{"line_number":132,"context_line":"        self.assertEqual(None, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":133,"context_line":"        self.leaseState.save()"},{"line_number":134,"context_line":"        self.assertEqual(\u0027CREATE\u0027, self.db_api.lease_get(1)[\u0027action\u0027])"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_save_state_with_nonexisting_lease(self):"},{"line_number":137,"context_line":"        def fake_lease_update_raise(id, values):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bad4c104_d63e3929","line":134,"in_reply_to":"3a1371a2_2a045b7d","updated":"2014-04-29 14:30:13.000000000","message":"Done","commit_id":"ba86ec20312cff0bc9544279d4c23b437a9ff331"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"980340eafb1bddc3d921931bf4e7dd14e557e3e1","unresolved":false,"context_lines":[{"line_number":29,"context_line":"                                             \u0027foo\u0027: \u0027bar\u0027,"},{"line_number":30,"context_line":"                                             \u0027action\u0027: None,"},{"line_number":31,"context_line":"                                             \u0027status\u0027: None,"},{"line_number":32,"context_line":"                                             \u0027status_reason\u0027: None})"},{"line_number":33,"context_line":"        self.lease.update(kw)"},{"line_number":34,"context_line":"        return self.lease"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_77cc8b36","line":32,"updated":"2014-04-30 10:45:45.000000000","message":"no need to use attribute self.lease, why no just using a local var ? and btw remove the call to getattr.","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7494,"name":"Guangya Liu","email":"gyliu513@gmail.com","username":"JayLau"},"change_message_id":"37f1c828446eb4f1af6f290a551644d9c546b5a0","unresolved":false,"context_lines":[{"line_number":29,"context_line":"                                             \u0027foo\u0027: \u0027bar\u0027,"},{"line_number":30,"context_line":"                                             \u0027action\u0027: None,"},{"line_number":31,"context_line":"                                             \u0027status\u0027: None,"},{"line_number":32,"context_line":"                                             \u0027status_reason\u0027: None})"},{"line_number":33,"context_line":"        self.lease.update(kw)"},{"line_number":34,"context_line":"        return self.lease"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_96742837","line":32,"in_reply_to":"9ad9bd40_77cc8b36","updated":"2014-05-06 09:17:22.000000000","message":"I think that we may have more test cases need fake lease, so can you please create a new file such as fake_lease.py just like fake_policy.py?","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"},{"author":{"_account_id":7075,"name":"Swann Croiset","email":"scroiset@mirantis.com","username":"swann"},"change_message_id":"be9e0047a5993055986fe0a9b8bc6e27550adb99","unresolved":false,"context_lines":[{"line_number":29,"context_line":"                                             \u0027foo\u0027: \u0027bar\u0027,"},{"line_number":30,"context_line":"                                             \u0027action\u0027: None,"},{"line_number":31,"context_line":"                                             \u0027status\u0027: None,"},{"line_number":32,"context_line":"                                             \u0027status_reason\u0027: None})"},{"line_number":33,"context_line":"        self.lease.update(kw)"},{"line_number":34,"context_line":"        return self.lease"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_1e902a49","line":32,"in_reply_to":"9ad9bd40_96742837","updated":"2014-05-06 12:41:02.000000000","message":"+1 to externalize our data test .. and it\u0027s cheap","commit_id":"a77e50a4b16918fd649fe053997d17788f678330"}]}