)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement a periodic task in the ConductorManager which will"},{"line_number":10,"context_line":"check all the existing, non-locked nodes and verify their power states."},{"line_number":11,"context_line":"For now, log a warning and update the db."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This is a step towards keeping powered-off-nodes off."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"AAAATn%2F%2B62Y%3D","line":11,"updated":"2013-11-18 17:25:50.000000000","message":"1. I think it would be great to expand this a little - something like \"...and verify that their current power states correspond to ones stored in DB. If they differ, for now, log a warning and update the DB using current power state.\"\n\n2. Would you please mention that this patch also handles swapping of get_node_list and get_nodes methods","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement a periodic task in the ConductorManager which will"},{"line_number":10,"context_line":"check all the existing, non-locked nodes and verify their power states."},{"line_number":11,"context_line":"For now, log a warning and update the db."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This is a step towards keeping powered-off-nodes off."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"AAAATn%2F%2BvXw%3D","line":11,"in_reply_to":"AAAATn%2F%2B62Y%3D","updated":"2013-11-21 22:31:09.000000000","message":"Done","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"}],"etc/ironic/ironic.conf.sample":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9b7d4741938c67c0f0aa5cb7bdd85f535f6005b7","unresolved":false,"context_lines":[{"line_number":431,"context_line":"# value)"},{"line_number":432,"context_line":"#heartbeat_timeout\u003d60"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":"# Time between sync_power_state checks. (integer value)"},{"line_number":435,"context_line":"#sync_power_state_interval\u003d60"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":13,"id":"AAAATn%2F94PY%3D","line":434,"updated":"2013-12-04 23:09:08.000000000","message":"Hey, isn\u0027t this generated from the code? manager.py shows a different help string.","commit_id":"18db8291ab828e673926a07707bd01149c88bfd4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"bb81c5890714f35b80e50793f537ac2abf096170","unresolved":false,"context_lines":[{"line_number":431,"context_line":"# value)"},{"line_number":432,"context_line":"#heartbeat_timeout\u003d60"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":"# Time between sync_power_state checks. (integer value)"},{"line_number":435,"context_line":"#sync_power_state_interval\u003d60"},{"line_number":436,"context_line":""},{"line_number":437,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":13,"id":"AAAATn%2F93%2Fk%3D","line":434,"in_reply_to":"AAAATn%2F94PY%3D","updated":"2013-12-04 23:48:55.000000000","message":"Graaah! Thanks for spotting that. I fixed the help string based on lifeless\u0027 comment in a prior review, but forgot to regenerate the conf.sample.....","commit_id":"18db8291ab828e673926a07707bd01149c88bfd4"}],"ironic/api/controllers/v1/node.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4308f733e64f07e49acb6db4608f7b29be866ca1","unresolved":false,"context_lines":[{"line_number":343,"context_line":"        else:"},{"line_number":344,"context_line":"            nodes \u003d pecan.request.dbapi.get_node_list(limit, marker_obj,"},{"line_number":345,"context_line":"                                                  sort_key\u003dsort_key,"},{"line_number":346,"context_line":"                                                  sort_dir\u003dsort_dir)"},{"line_number":347,"context_line":"        return nodes"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    def _get_nodes_by_instance(self, instance_uuid):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BnGc%3D","line":346,"updated":"2013-11-22 15:13:54.000000000","message":"Please re indent it again :) (I understand it was renamed back from get_nodes())","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3576844ab47bf1fddff29dc0e2173cb4faf66a9d","unresolved":false,"context_lines":[{"line_number":343,"context_line":"        else:"},{"line_number":344,"context_line":"            nodes \u003d pecan.request.dbapi.get_node_list(limit, marker_obj,"},{"line_number":345,"context_line":"                                                  sort_key\u003dsort_key,"},{"line_number":346,"context_line":"                                                  sort_dir\u003dsort_dir)"},{"line_number":347,"context_line":"        return nodes"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    def _get_nodes_by_instance(self, instance_uuid):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BlV0%3D","line":346,"in_reply_to":"AAAATn%2F%2BnGc%3D","updated":"2013-11-22 20:07:35.000000000","message":"Done","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"}],"ironic/conductor/manager.py":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4fc648a0a97fd3ea820511c92aa8da6f93d7753f","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        filters \u003d {\u0027reserved\u0027: False}"},{"line_number":323,"context_line":"        # TODO(deva): add filter by conductor"},{"line_number":324,"context_line":"        node_list \u003d self.dbapi.get_node_list(filters\u003dfilters)"},{"line_number":325,"context_line":"        for node_id in [n[0] for n in node_list]:"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FCwo%3D","line":325,"updated":"2013-11-15 08:50:07.000000000","message":"n[\u0027id\u0027] instead of n[0]?","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"0df12bf244233ffa3c275389c6391f4b320e9e5c","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        filters \u003d {\u0027reserved\u0027: False}"},{"line_number":323,"context_line":"        # TODO(deva): add filter by conductor"},{"line_number":324,"context_line":"        node_list \u003d self.dbapi.get_node_list(filters\u003dfilters)"},{"line_number":325,"context_line":"        for node_id in [n[0] for n in node_list]:"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FA4c%3D","line":325,"in_reply_to":"AAAATn%2F%2FA60%3D","updated":"2013-11-15 17:35:08.000000000","message":"Actually, get_node_list returns a tuple, not a dict, which, I think, is a result of the way sqlalchemy works when fetching only specific columns. String indexing won\u0027t work; this will have to remain n[0]","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"7ec8cac7de56821c54326488f7e9496043da8709","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        filters \u003d {\u0027reserved\u0027: False}"},{"line_number":323,"context_line":"        # TODO(deva): add filter by conductor"},{"line_number":324,"context_line":"        node_list \u003d self.dbapi.get_node_list(filters\u003dfilters)"},{"line_number":325,"context_line":"        for node_id in [n[0] for n in node_list]:"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FA60%3D","line":325,"in_reply_to":"AAAATn%2F%2FCwo%3D","updated":"2013-11-15 17:25:24.000000000","message":"Explicit is better. I\u0027ll change it :)","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4fc648a0a97fd3ea820511c92aa8da6f93d7753f","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                                       \u0027old_state\u0027: node[\u0027power_state\u0027],"},{"line_number":336,"context_line":"                                       \u0027new_state\u0027: power_state})"},{"line_number":337,"context_line":"                        node[\u0027power_state\u0027] \u003d power_state"},{"line_number":338,"context_line":"                        node.save()"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"            # skip locked nodes or nodes deleted during the loop"},{"line_number":341,"context_line":"            except (exception.NodeNotFound, exception.NodeLocked):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FCxc%3D","line":338,"updated":"2013-11-15 08:50:07.000000000","message":"Devananda, what you think about node.save(context) or node.save(task.context) w/o\na hack?","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"7ec8cac7de56821c54326488f7e9496043da8709","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                                       \u0027old_state\u0027: node[\u0027power_state\u0027],"},{"line_number":336,"context_line":"                                       \u0027new_state\u0027: power_state})"},{"line_number":337,"context_line":"                        node[\u0027power_state\u0027] \u003d power_state"},{"line_number":338,"context_line":"                        node.save()"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"            # skip locked nodes or nodes deleted during the loop"},{"line_number":341,"context_line":"            except (exception.NodeNotFound, exception.NodeLocked):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FA6s%3D","line":338,"in_reply_to":"AAAATn%2F%2FCxc%3D","updated":"2013-11-15 17:25:24.000000000","message":"I think that\u0027s reasonable.","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        filters \u003d {\u0027reserved\u0027: False}"},{"line_number":323,"context_line":"        # TODO(deva): add filter by conductor"},{"line_number":324,"context_line":"        node_list \u003d self.dbapi.get_node_list(filters\u003dfilters)"},{"line_number":325,"context_line":"        for node_id in [n[0] for n in node_list]:"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6uo%3D","line":325,"updated":"2013-11-18 17:25:50.000000000","message":"As an option - we can replace this with tuple assignment:\n\n  for node_id, in node_list:\nor\n  for (node_id,) in node_list:","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        filters \u003d {\u0027reserved\u0027: False}"},{"line_number":323,"context_line":"        # TODO(deva): add filter by conductor"},{"line_number":324,"context_line":"        node_list \u003d self.dbapi.get_node_list(filters\u003dfilters)"},{"line_number":325,"context_line":"        for node_id in [n[0] for n in node_list]:"},{"line_number":326,"context_line":"            try:"},{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BqQI%3D","line":325,"in_reply_to":"AAAATn%2F%2B6uo%3D","updated":"2013-11-21 22:31:09.000000000","message":"Ah, nice short cut. thanks","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"},{"line_number":329,"context_line":"                    power_state \u003d task.driver.power.get_power_state(task, node)"},{"line_number":330,"context_line":"                    if power_state !\u003d node[\u0027power_state\u0027]:"},{"line_number":331,"context_line":"                        LOG.warning(_(\"Node %(node)s last known power state: \""},{"line_number":332,"context_line":"                                      \"%(old_state)s. Current power state: \""},{"line_number":333,"context_line":"                                      \"%(new_state)s. Updating DB.\") %"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6f8%3D","line":330,"updated":"2013-11-18 17:25:50.000000000","message":"I see there is a comment https://github.com/openstack/ironic/blob/master/ironic/objects/base.py#L342\nthat dict-based access may be removed in future.. Will it better to use node.power_state here, or there are some other points that I\u0027m missing? \n\nThe remaining code use this as well, but maybe it\u0027s time to start the new approach? \u003d)","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"},{"line_number":329,"context_line":"                    power_state \u003d task.driver.power.get_power_state(task, node)"},{"line_number":330,"context_line":"                    if power_state !\u003d node[\u0027power_state\u0027]:"},{"line_number":331,"context_line":"                        LOG.warning(_(\"Node %(node)s last known power state: \""},{"line_number":332,"context_line":"                                      \"%(old_state)s. Current power state: \""},{"line_number":333,"context_line":"                                      \"%(new_state)s. Updating DB.\") %"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BqG4%3D","line":330,"in_reply_to":"AAAATn%2F%2B6f8%3D","updated":"2013-11-21 22:31:09.000000000","message":"since this is a fairly small addition, I\u0027d rather maintain consistency within the file. I\u0027m happy for it to be cleaned up, though, or for new files to access these as attributes instead of dicts.","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"b26fc747362aef0cfe87a99835861764e88a267c","unresolved":false,"context_lines":[{"line_number":327,"context_line":"                with task_manager.acquire(context, node_id) as task:"},{"line_number":328,"context_line":"                    node \u003d task.node"},{"line_number":329,"context_line":"                    power_state \u003d task.driver.power.get_power_state(task, node)"},{"line_number":330,"context_line":"                    if power_state !\u003d node[\u0027power_state\u0027]:"},{"line_number":331,"context_line":"                        LOG.warning(_(\"Node %(node)s last known power state: \""},{"line_number":332,"context_line":"                                      \"%(old_state)s. Current power state: \""},{"line_number":333,"context_line":"                                      \"%(new_state)s. Updating DB.\") %"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Bn8s%3D","line":330,"in_reply_to":"AAAATn%2F%2BqG4%3D","updated":"2013-11-22 10:03:51.000000000","message":"Agree","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":334,"context_line":"                                      {\u0027node\u0027: node[\u0027uuid\u0027],"},{"line_number":335,"context_line":"                                       \u0027old_state\u0027: node[\u0027power_state\u0027],"},{"line_number":336,"context_line":"                                       \u0027new_state\u0027: power_state})"},{"line_number":337,"context_line":"                        node[\u0027power_state\u0027] \u003d power_state"},{"line_number":338,"context_line":"                        node.save(context)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"            # skip locked nodes or nodes deleted during the loop"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6f4%3D","line":337,"updated":"2013-11-18 17:25:50.000000000","message":"ditto regarding dict-based access","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                        node[\u0027power_state\u0027] \u003d power_state"},{"line_number":338,"context_line":"                        node.save(context)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"            # skip locked nodes or nodes deleted during the loop"},{"line_number":341,"context_line":"            except (exception.NodeNotFound, exception.NodeLocked):"},{"line_number":342,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6mo%3D","line":340,"updated":"2013-11-18 17:25:50.000000000","message":"Probably a better wording - # skip nodes that were locked or deleted during the loop","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                        node[\u0027power_state\u0027] \u003d power_state"},{"line_number":338,"context_line":"                        node.save(context)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"            # skip locked nodes or nodes deleted during the loop"},{"line_number":341,"context_line":"            except (exception.NodeNotFound, exception.NodeLocked):"},{"line_number":342,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BqPE%3D","line":340,"in_reply_to":"AAAATn%2F%2B6mo%3D","updated":"2013-11-21 22:31:09.000000000","message":"Done","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"33eec986f5bb78a297d319d934528134434900d1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                   help\u003d\u0027Maximum time since the last check-in of a conductor\u0027),"},{"line_number":68,"context_line":"        cfg.IntOpt(\u0027sync_power_state_interval\u0027,"},{"line_number":69,"context_line":"                   default\u003d60,"},{"line_number":70,"context_line":"                   help\u003d\u0027Time between sync_power_state checks.\u0027),"},{"line_number":71,"context_line":"]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F99ig%3D","line":70,"updated":"2013-12-03 23:53:47.000000000","message":"Could you add units here? e.g. either sync_power_state_seconds or in the help.","commit_id":"8d14f7d41831b6d39e58442ee5b0a9a499ee5757"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"c83afa48c04ae001d3a8ed4c865cb564045d65b2","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                   help\u003d\u0027Maximum time since the last check-in of a conductor\u0027),"},{"line_number":68,"context_line":"        cfg.IntOpt(\u0027sync_power_state_interval\u0027,"},{"line_number":69,"context_line":"                   default\u003d60,"},{"line_number":70,"context_line":"                   help\u003d\u0027Time between sync_power_state checks.\u0027),"},{"line_number":71,"context_line":"]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F99Y0%3D","line":70,"in_reply_to":"AAAATn%2F99ig%3D","updated":"2013-12-04 00:40:17.000000000","message":"Done","commit_id":"8d14f7d41831b6d39e58442ee5b0a9a499ee5757"},{"author":{"_account_id":5805,"name":"Chris Krelle","email":"nobodycam@gmail.com","username":"nobodycam"},"change_message_id":"c21edab71a6a05dfbd548b4e68ffde533725f457","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                    node \u003d task.node"},{"line_number":390,"context_line":"                    power_state \u003d task.driver.power.get_power_state(task, node)"},{"line_number":391,"context_line":"                    if power_state !\u003d node[\u0027power_state\u0027]:"},{"line_number":392,"context_line":"                        LOG.warning(_(\"Node %(node)s last known power state: \""},{"line_number":393,"context_line":"                                      \"%(old_state)s. Current power state: \""},{"line_number":394,"context_line":"                                      \"%(new_state)s. Updating DB.\") %"},{"line_number":395,"context_line":"                                      {\u0027node\u0027: node[\u0027uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F99qk%3D","line":392,"updated":"2013-12-03 23:32:33.000000000","message":"very minor nit. I would like to see LOG.audit vs warning here.","commit_id":"8d14f7d41831b6d39e58442ee5b0a9a499ee5757"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"c83afa48c04ae001d3a8ed4c865cb564045d65b2","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                    node \u003d task.node"},{"line_number":390,"context_line":"                    power_state \u003d task.driver.power.get_power_state(task, node)"},{"line_number":391,"context_line":"                    if power_state !\u003d node[\u0027power_state\u0027]:"},{"line_number":392,"context_line":"                        LOG.warning(_(\"Node %(node)s last known power state: \""},{"line_number":393,"context_line":"                                      \"%(old_state)s. Current power state: \""},{"line_number":394,"context_line":"                                      \"%(new_state)s. Updating DB.\") %"},{"line_number":395,"context_line":"                                      {\u0027node\u0027: node[\u0027uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F99WI%3D","line":392,"in_reply_to":"AAAATn%2F99qk%3D","updated":"2013-12-04 00:40:17.000000000","message":"Thanks. I\u0027ll add an if-not-None-then-warning here.","commit_id":"8d14f7d41831b6d39e58442ee5b0a9a499ee5757"}],"ironic/db/api.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"1beda344a041dc5e98b1b62276f0823c47f41319","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B4R0%3D","line":45,"updated":"2013-11-19 01:54:39.000000000","message":"I like get_node_list() returning a list of Node objects. If we change it to get_nodes(), should we be consistent and change eg get_port_list() to get_ports() too?\n\nAll the other \u0027node\u0027-related methods here deal with lists of Nodes or lists of node ids/uuids. This new get_node_list() seems like the odd-method-out, because it deals with lists of tuples. I don\u0027t think it makes sense to call it get_nodetuples_list, but what about something like get_nodeinfo_list() or get_node_cols_list()? \n\nWe\u0027re fine with returning a tuple, right? vs eg a dict of column names : values? (I think yes, but thought I\u0027d ask anyway.)","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BvV8%3D","line":45,"in_reply_to":"AAAATn%2F%2B4R0%3D","updated":"2013-11-21 22:31:09.000000000","message":"Hm, you\u0027re right. This was inconsistent of me. I\u0027ll delete get_nodes, since we haven\u0027t used it anywhere, and create a new method. \n\nHow is \"get_filtered_node_list\" for a name?","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"b26fc747362aef0cfe87a99835861764e88a267c","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Bn8w%3D","line":45,"in_reply_to":"AAAATn%2F%2BvV8%3D","updated":"2013-11-22 10:03:51.000000000","message":"I like get_nodeinfo_list(), and if we do that, then get_node_list will be OK too. Both of them will support filtering so I think we may not mention this in method name.","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"26f9aa82afd72ab1507b29fd563f0dc61e81ab92","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_filtered_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Bn6U%3D","line":45,"updated":"2013-11-22 10:38:28.000000000","message":"As per previous review, this were suggested to be renamed to get_nodeinfo_list(); get_node_list leave as is. After refactoring both of them will support filtering so I think we may not mention this in method name.\n\nBut we can rename during the refactoring. Up to you","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4308f733e64f07e49acb6db4608f7b29be866ca1","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_filtered_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Bm64%3D","line":45,"updated":"2013-11-22 15:13:54.000000000","message":"Let\u0027s not use lists (or any other mutable objects like dicts and sets) as defaults arguments values, it\u0027s a bad practice and dangerous because modification on these objects will remain across multiple calls.\n\nhttp://python.net/~goodger/projects/pycon/2007/idiomatic/handout.html#default-parameter-values\n\nI also agree with Max that the name of this method is a sort of misleading since get_node_list() also does filtering.","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"762164bd6e696dd5cf2ba7a42a5536fa431c2edf","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_filtered_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BfrY%3D","line":45,"in_reply_to":"AAAATn%2F%2BlVY%3D","updated":"2013-11-25 14:13:45.000000000","message":"I see what you mean, I was considering pagination (limiting the size of the output) as a sort of filter, but yea I can also see that it might differ from the type of filtering that this new method is doing.","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3576844ab47bf1fddff29dc0e2173cb4faf66a9d","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        \"\"\"Constructor.\"\"\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @abc.abstractmethod"},{"line_number":45,"context_line":"    def get_filtered_node_list(self, columns\u003d[], filters\u003d[],"},{"line_number":46,"context_line":"                  limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":47,"context_line":"        \"\"\"Return a list of tuples of the specified columns."},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BlVY%3D","line":45,"in_reply_to":"AAAATn%2F%2Bm64%3D","updated":"2013-11-22 20:07:35.000000000","message":"Done re: no-lists-as-default.\n\n@Lucas, I don\u0027t see how get_node_list() does any filtering today. It does sorting and pagination. I also don\u0027t see any reviews up that will add this, so I\u0027m not sure what you\u0027re referring to.","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"12e05373e302848fef4bbc158e72128542f318da","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        :param sort_key: Attribute by which results should be sorted."},{"line_number":59,"context_line":"        :param sort_dir: direction in which results should be sorted."},{"line_number":60,"context_line":"                         (asc, desc)"},{"line_number":61,"context_line":"        :returns: A list of lists."},{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BYyU%3D","line":61,"updated":"2013-11-26 16:27:34.000000000","message":"I thought it returned a list of tuples?","commit_id":"3f39a9089b6c095d9073eb756f7fa5edc202c69e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d8fbb2ca0ad06843a6f558e380da7514a7174167","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        :param sort_key: Attribute by which results should be sorted."},{"line_number":59,"context_line":"        :param sort_dir: direction in which results should be sorted."},{"line_number":60,"context_line":"                         (asc, desc)"},{"line_number":61,"context_line":"        :returns: A list of lists."},{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2BR60%3D","line":61,"updated":"2013-11-27 22:51:36.000000000","message":"Hi Deva, sorry, you may have missed my question in a previous patch. Doesn\u0027t it return a list of tuples, not lists?","commit_id":"ffcf95040d3ad319ac335de67606d4db6b3981bb"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b977e22a748f8fe22f59c5839ee349656aec80de","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        :param sort_key: Attribute by which results should be sorted."},{"line_number":59,"context_line":"        :param sort_dir: direction in which results should be sorted."},{"line_number":60,"context_line":"                         (asc, desc)"},{"line_number":61,"context_line":"        :returns: A list of lists."},{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2BR3c%3D","line":61,"in_reply_to":"AAAATn%2F%2BR60%3D","updated":"2013-11-27 23:11:11.000000000","message":"Done","commit_id":"ffcf95040d3ad319ac335de67606d4db6b3981bb"}],"ironic/db/sqlalchemy/api.py":[{"author":{"_account_id":1726,"name":"Ghe Rivero","email":"ghe.rivero@gmail.com","username":"ghe.rivero"},"change_message_id":"37f59d48dceecb77272931f651cebd7815654336","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def get_node_list(self, columns\u003d[models.Node.id], filters\u003d[],"},{"line_number":183,"context_line":"                      limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":184,"context_line":"        query \u003d model_query(*columns, base_model\u003dmodels.Node)"},{"line_number":185,"context_line":"        if \u0027associated\u0027 in filters:"},{"line_number":186,"context_line":"            if filters[\u0027associated\u0027]:"},{"line_number":187,"context_line":"                query \u003d query.filter(models.Node.instance_uuid !\u003d None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FCNk%3D","line":184,"updated":"2013-11-15 11:35:04.000000000","message":"def model_query(model, *args, **kwargs) and the only kwargs used is \u0027session\u0027, so this should be:\n\nquery \u003d model_query(models.Node, *columns)","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"7ec8cac7de56821c54326488f7e9496043da8709","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def get_node_list(self, columns\u003d[models.Node.id], filters\u003d[],"},{"line_number":183,"context_line":"                      limit\u003dNone, marker\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":184,"context_line":"        query \u003d model_query(*columns, base_model\u003dmodels.Node)"},{"line_number":185,"context_line":"        if \u0027associated\u0027 in filters:"},{"line_number":186,"context_line":"            if filters[\u0027associated\u0027]:"},{"line_number":187,"context_line":"                query \u003d query.filter(models.Node.instance_uuid !\u003d None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2FA68%3D","line":184,"in_reply_to":"AAAATn%2F%2FCNk%3D","updated":"2013-11-15 17:25:24.000000000","message":"I specifically do not want this method fetching entire Node record -- it should only return the specified columns. So first argument should not be models.Node.","commit_id":"1738d6f27f9b44cf7b9a4c651778f58e5c7f3125"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                query \u003d query.filter(models.Node.reservation \u003d\u003d None)"},{"line_number":195,"context_line":"        # TODO(deva): add possibility to filter by conductor"},{"line_number":196,"context_line":"        return _paginate_query(models.Node, limit, marker,"},{"line_number":197,"context_line":"                               sort_key, sort_dir, query)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @objects.objectify(objects.Node)"},{"line_number":200,"context_line":"    def get_nodes(self, limit\u003dNone, marker\u003dNone,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6X0%3D","line":197,"updated":"2013-11-18 17:25:50.000000000","message":"We could also re-implement get_nodes to be similar to get_node_list, we will be able to remove get_associated_nodes, get_unassociated_nodes. Also as an option during the refactoring we could add there a chassis filter and then remove \nget_nodes_by_chassis as well.\n\nSingle method with a bunch of supported filters seems more flexible to me, we could stick with this approach for both get_node_list and get_nodes. Probably will need to extract all filter-handling logic to a separate method to reuse in these two. Pls let me know if you agree, I\u0027ll do the refactoring once this landed","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                query \u003d query.filter(models.Node.reservation \u003d\u003d None)"},{"line_number":195,"context_line":"        # TODO(deva): add possibility to filter by conductor"},{"line_number":196,"context_line":"        return _paginate_query(models.Node, limit, marker,"},{"line_number":197,"context_line":"                               sort_key, sort_dir, query)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @objects.objectify(objects.Node)"},{"line_number":200,"context_line":"    def get_nodes(self, limit\u003dNone, marker\u003dNone,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BvVo%3D","line":197,"in_reply_to":"AAAATn%2F%2B6X0%3D","updated":"2013-11-21 22:31:09.000000000","message":"Yea, as I sketched this out, I realized that a few other methods could be similarly refactored. I think that\u0027s fine.","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4308f733e64f07e49acb6db4608f7b29be866ca1","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def get_filtered_node_list(self, columns\u003d[models.Node.id], filters\u003d[],"},{"line_number":183,"context_line":"                               limit\u003dNone, marker\u003dNone, sort_key\u003dNone,"},{"line_number":184,"context_line":"                               sort_dir\u003dNone):"},{"line_number":185,"context_line":"        query \u003d model_query(*columns, base_model\u003dmodels.Node)"},{"line_number":186,"context_line":"        if \u0027associated\u0027 in filters:"},{"line_number":187,"context_line":"            if filters[\u0027associated\u0027]:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Bm24%3D","line":184,"updated":"2013-11-22 15:13:54.000000000","message":"ditto about lists as default values","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3576844ab47bf1fddff29dc0e2173cb4faf66a9d","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def get_filtered_node_list(self, columns\u003d[models.Node.id], filters\u003d[],"},{"line_number":183,"context_line":"                               limit\u003dNone, marker\u003dNone, sort_key\u003dNone,"},{"line_number":184,"context_line":"                               sort_dir\u003dNone):"},{"line_number":185,"context_line":"        query \u003d model_query(*columns, base_model\u003dmodels.Node)"},{"line_number":186,"context_line":"        if \u0027associated\u0027 in filters:"},{"line_number":187,"context_line":"            if filters[\u0027associated\u0027]:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2BlVI%3D","line":184,"in_reply_to":"AAAATn%2F%2Bm24%3D","updated":"2013-11-22 20:07:35.000000000","message":"Done","commit_id":"8c379a1f77eb0fc99ba955c94b4768fe438c3133"}],"ironic/tests/conductor/test_manager.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"1beda344a041dc5e98b1b62276f0823c47f41319","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        with mock.patch.object(self.dbapi, \u0027touch_conductor\u0027) as mock_touch:"},{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_once_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_get_power_state(self):"},{"line_number":79,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027)"},{"line_number":80,"context_line":"        self.dbapi.create_node(n)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B4SI%3D","side":"PARENT","line":77,"updated":"2013-11-19 01:54:39.000000000","message":"What about a test or modifying this test, with two nodes, the first locked and the 2nd being sync\u0027d. Just to make sure your loop works :-)","commit_id":"8bba5fd6af7a47ce201ce5034e42eacf27d990fd"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        with mock.patch.object(self.dbapi, \u0027touch_conductor\u0027) as mock_touch:"},{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_once_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_get_power_state(self):"},{"line_number":79,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027)"},{"line_number":80,"context_line":"        self.dbapi.create_node(n)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BqRI%3D","side":"PARENT","line":77,"in_reply_to":"AAAATn%2F%2B4SI%3D","updated":"2013-11-21 22:31:09.000000000","message":"Done","commit_id":"8bba5fd6af7a47ce201ce5034e42eacf27d990fd"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.service.start()"},{"line_number":74,"context_line":"        with mock.patch.object(self.dbapi, \u0027touch_conductor\u0027) as mock_touch:"},{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6VA%3D","line":76,"updated":"2013-11-18 17:25:50.000000000","message":"Sorry, I can\u0027t find a code change related to this test update. Would you please explain a little?","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.service.start()"},{"line_number":74,"context_line":"        with mock.patch.object(self.dbapi, \u0027touch_conductor\u0027) as mock_touch:"},{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BvVQ%3D","line":76,"in_reply_to":"AAAATn%2F%2B6VA%3D","updated":"2013-11-21 22:31:09.000000000","message":"Darn, you caught me.\n\nActually there was an odd issue -- as soon as I added a second periodic_task, this test started failing because dbapi.touch_conductor() was getting called TWICE!\n\nI haven\u0027t dug into why that is. I\u0027ll file a bug and note it in the next revision.\n#1253426","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"b26fc747362aef0cfe87a99835861764e88a267c","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.service.start()"},{"line_number":74,"context_line":"        with mock.patch.object(self.dbapi, \u0027touch_conductor\u0027) as mock_touch:"},{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Bn%2BU%3D","line":76,"in_reply_to":"AAAATn%2F%2BvVQ%3D","updated":"2013-11-22 10:03:51.000000000","message":"hehe \u003d) thanks","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"},{"line_number":80,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, power_state\u003d\u0027fake-power\u0027)"},{"line_number":81,"context_line":"        self.dbapi.create_node(n)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6Qw%3D","line":78,"updated":"2013-11-18 17:25:50.000000000","message":"Will it better to name it \n  test_periodic_sync_power_state_no_sync\nor\n  test_periodic_sync_power_state_no_need_to_sync","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            self.service.periodic_tasks(self.context)"},{"line_number":76,"context_line":"            mock_touch.assert_called_with(\u0027test-host\u0027)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"},{"line_number":80,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, power_state\u003d\u0027fake-power\u0027)"},{"line_number":81,"context_line":"        self.dbapi.create_node(n)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BvSI%3D","line":78,"in_reply_to":"AAAATn%2F%2B6Qw%3D","updated":"2013-11-21 22:31:09.000000000","message":"Done","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"},{"line_number":80,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, power_state\u003d\u0027fake-power\u0027)"},{"line_number":81,"context_line":"        self.dbapi.create_node(n)"},{"line_number":82,"context_line":"        with mock.patch.object(self.driver.power,"},{"line_number":83,"context_line":"                               \u0027get_power_state\u0027) as get_power_mock:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6QI%3D","line":80,"updated":"2013-11-18 17:25:50.000000000","message":"Since this test covers case when we have consistent DB and bare-metal states, maybe it\u0027s better to use some valid state here? Same starting and resulting \u0027fake-power\u0027 confused me a little","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"},{"line_number":80,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, power_state\u003d\u0027fake-power\u0027)"},{"line_number":81,"context_line":"        self.dbapi.create_node(n)"},{"line_number":82,"context_line":"        with mock.patch.object(self.driver.power,"},{"line_number":83,"context_line":"                               \u0027get_power_state\u0027) as get_power_mock:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BvSM%3D","line":80,"in_reply_to":"AAAATn%2F%2B6QI%3D","updated":"2013-11-21 22:31:09.000000000","message":"I chose to use a fake state here so it is clear that it is not being altered by the conductor or driver, or even being compared to a valid state.","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"b26fc747362aef0cfe87a99835861764e88a267c","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_periodic_sync_power_state_ok(self):"},{"line_number":79,"context_line":"        self.service.start()"},{"line_number":80,"context_line":"        n \u003d utils.get_test_node(driver\u003d\u0027fake\u0027, power_state\u003d\u0027fake-power\u0027)"},{"line_number":81,"context_line":"        self.dbapi.create_node(n)"},{"line_number":82,"context_line":"        with mock.patch.object(self.driver.power,"},{"line_number":83,"context_line":"                               \u0027get_power_state\u0027) as get_power_mock:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Bn%2BA%3D","line":80,"in_reply_to":"AAAATn%2F%2BvSM%3D","updated":"2013-11-22 10:03:51.000000000","message":"Ok, now it seems more reasonable to me.","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        self.dbapi.create_node(n)"},{"line_number":82,"context_line":"        with mock.patch.object(self.driver.power,"},{"line_number":83,"context_line":"                               \u0027get_power_state\u0027) as get_power_mock:"},{"line_number":84,"context_line":"            get_power_mock.side_effect \u003d [\u0027fake-power\u0027]"},{"line_number":85,"context_line":"            self.service._sync_power_states(self.context)"},{"line_number":86,"context_line":"            get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":87,"context_line":"        node \u003d self.dbapi.get_node(n[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6Sk%3D","line":84,"updated":"2013-11-18 17:25:50.000000000","message":"as I understand side_effect is used here to set mock\u0027s return value. Would it better to use get_power_mock.return_value directly to avoid misunderstandings here? Mock man ref http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.return_value","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        self.dbapi.create_node(n)"},{"line_number":82,"context_line":"        with mock.patch.object(self.driver.power,"},{"line_number":83,"context_line":"                               \u0027get_power_state\u0027) as get_power_mock:"},{"line_number":84,"context_line":"            get_power_mock.side_effect \u003d [\u0027fake-power\u0027]"},{"line_number":85,"context_line":"            self.service._sync_power_states(self.context)"},{"line_number":86,"context_line":"            get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":87,"context_line":"        node \u003d self.dbapi.get_node(n[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BvSQ%3D","line":84,"in_reply_to":"AAAATn%2F%2B6Sk%3D","updated":"2013-11-21 22:31:09.000000000","message":"Yep.","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"000bdb269b3eccadcccbaf1ba8ccfcd5d4b412c5","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        self.dbapi.create_node(n)"},{"line_number":94,"context_line":"        with mock.patch.object(self.driver.power,"},{"line_number":95,"context_line":"                               \u0027get_power_state\u0027) as get_power_mock:"},{"line_number":96,"context_line":"            get_power_mock.side_effect \u003d [states.POWER_ON]"},{"line_number":97,"context_line":"            self.service._sync_power_states(self.context)"},{"line_number":98,"context_line":"            get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":99,"context_line":"        node \u003d self.dbapi.get_node(n[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B6Pk%3D","line":96,"updated":"2013-11-18 17:25:50.000000000","message":"ditto return_value","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"35b793f2af0769c72b58548b87d3d8745a710b5b","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import mock"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from ironic.openstack.common import uuidutils"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from ironic.common import driver_factory"},{"line_number":27,"context_line":"from ironic.common import exception"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BWdA%3D","line":24,"updated":"2013-11-27 02:18:38.000000000","message":"Devananda, just a kindly reminder, maybe this import is out-of-date, the uuidutils from oslo has been removed from our Ironic already I think.","commit_id":"3f39a9089b6c095d9073eb756f7fa5edc202c69e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"12e05373e302848fef4bbc158e72128542f318da","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import mock"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from ironic.openstack.common import uuidutils"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from ironic.common import driver_factory"},{"line_number":27,"context_line":"from ironic.common import exception"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BY7o%3D","line":24,"updated":"2013-11-26 16:27:34.000000000","message":"Those that are picky (not me), might wonder why this is not next to ironic.openstack.common.context below.","commit_id":"3f39a9089b6c095d9073eb756f7fa5edc202c69e"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3a48465307b88af03e8207e53be1f755f0a113b3","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import mock"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from ironic.openstack.common import uuidutils"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from ironic.common import driver_factory"},{"line_number":27,"context_line":"from ironic.common import exception"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BSQw%3D","line":24,"in_reply_to":"AAAATn%2F%2BWdA%3D","updated":"2013-11-27 21:07:25.000000000","message":"Done","commit_id":"3f39a9089b6c095d9073eb756f7fa5edc202c69e"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"92f1d00855c43f9f8666152ef0ac9deb379b755b","unresolved":false,"context_lines":[{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        # create three nodes"},{"line_number":119,"context_line":"        nodes \u003d []"},{"line_number":120,"context_line":"        for i in xrange(0, 3):"},{"line_number":121,"context_line":"            n \u003d db_utils.get_test_node(id\u003di, uuid\u003dutils.generate_uuid(),"},{"line_number":122,"context_line":"                    driver\u003d\u0027fake\u0027, power_state\u003dstates.POWER_OFF)"},{"line_number":123,"context_line":"            self.dbapi.create_node(n)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2BID0%3D","line":120,"updated":"2013-12-02 10:23:05.000000000","message":"Let\u0027s use range() here, xrange() doesn\u0027t exist in py3 (range() in python2 is ok if the list is not very large where memory issues might start appearing).\n\nAlso, we currently have a patch being reviewed that\u0027s replacing all xrange\u0027s from the ironic code[1] and it\u0027s marking a bug as closed so it\u0027s better to not introduce a new xrange in the code.\n\n[1] https://review.openstack.org/#/c/59114/","commit_id":"2316a90ef1b783377b7d9e41fc9efbb60ce06f73"}],"ironic/tests/db/test_nodes.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"1beda344a041dc5e98b1b62276f0823c47f41319","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    # TODO(deva): add more tests for get_node_list"},{"line_number":120,"context_line":"    #             - check each filter works"},{"line_number":121,"context_line":"    #             - check variable column return works"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def test_get_nodes(self):"},{"line_number":124,"context_line":"        uuids \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B4Sg%3D","line":121,"updated":"2013-11-19 01:54:39.000000000","message":"So when people put a \u0027TODO\u0027. Does it mean that they (or someone else) might DO in another code review, or in an updated patch for the same code review, or either, depending on the submitter\u0027s discretion?","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f9da307bd0c23aae2bcf0fc00ef6e08b84e7e29f","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    # TODO(deva): add more tests for get_node_list"},{"line_number":120,"context_line":"    #             - check each filter works"},{"line_number":121,"context_line":"    #             - check variable column return works"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def test_get_nodes(self):"},{"line_number":124,"context_line":"        uuids \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BqRE%3D","line":121,"in_reply_to":"AAAATn%2F%2B4Sg%3D","updated":"2013-11-21 22:31:09.000000000","message":"TODO usually means that it won\u0027t be done in the same review -- it\u0027s a way to annotate an intention for later.\n\nThe (name) indicates who wrote the TODO, not necessarily who is going to do it. But it often ends up being the person who wrote it :)","commit_id":"193c3bca00cf95295c47d7fc84e62d2d712a9ed2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"35b793f2af0769c72b58548b87d3d8745a710b5b","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"Tests for manipulating Nodes via the DB API\"\"\""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from ironic.openstack.common import uuidutils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from ironic.common import exception"},{"line_number":23,"context_line":"from ironic.db import api as dbapi"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BWc0%3D","line":20,"updated":"2013-11-27 02:18:38.000000000","message":"ditto","commit_id":"3f39a9089b6c095d9073eb756f7fa5edc202c69e"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"35b793f2af0769c72b58548b87d3d8745a710b5b","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def test_get_nodeinfo_list_defaults(self):"},{"line_number":113,"context_line":"        for i in xrange(1, 6):"},{"line_number":114,"context_line":"            n \u003d utils.get_test_node(id\u003di, uuid\u003duuidutils.generate_uuid())"},{"line_number":115,"context_line":"            self.dbapi.create_node(n)"},{"line_number":116,"context_line":"        res \u003d [i[0] for i in self.dbapi.get_nodeinfo_list()]"},{"line_number":117,"context_line":"        self.assertEqual(sorted(res), sorted(xrange(1, 6)))"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BWcw%3D","line":114,"updated":"2013-11-27 02:18:38.000000000","message":"ditto","commit_id":"3f39a9089b6c095d9073eb756f7fa5edc202c69e"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"92f1d00855c43f9f8666152ef0ac9deb379b755b","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                          self.dbapi.get_node, \u0027not-a-uuid\u0027)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def test_get_nodeinfo_list_defaults(self):"},{"line_number":116,"context_line":"        for i in xrange(1, 6):"},{"line_number":117,"context_line":"            n \u003d utils.get_test_node(id\u003di, uuid\u003dironic_utils.generate_uuid())"},{"line_number":118,"context_line":"            self.dbapi.create_node(n)"},{"line_number":119,"context_line":"        res \u003d [i[0] for i in self.dbapi.get_nodeinfo_list()]"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2BIDM%3D","line":116,"updated":"2013-12-02 10:23:05.000000000","message":"same as in test_manager.py, xrange is not py3 compatible, let\u0027s just use range()","commit_id":"2316a90ef1b783377b7d9e41fc9efbb60ce06f73"}]}