)]}'
{"tests/unit/test_scheduler.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"9592384131719e969c4d1b8692f04c1d376a515c","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"            held_node[\u0027hold_job\u0027],"},{"line_number":1460,"context_line":"            \" \".join([\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027])"},{"line_number":1461,"context_line":"        )"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"    @simple_layout(\u0027layouts/three-projects.yaml\u0027)"},{"line_number":1464,"context_line":"    def test_dependent_behind_dequeue(self):"},{"line_number":1465,"context_line":"        # This particular test does a large amount of merges and needs a little"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_65f48e5c","line":1462,"updated":"2017-07-28 19:37:14.000000000","message":"Maybe add another change and check that the second node is not held?","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"255c0de352ceb302d4256cc8a367f59e1b900a98","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"            held_node[\u0027hold_job\u0027],"},{"line_number":1460,"context_line":"            \" \".join([\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027])"},{"line_number":1461,"context_line":"        )"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"    @simple_layout(\u0027layouts/three-projects.yaml\u0027)"},{"line_number":1464,"context_line":"    def test_dependent_behind_dequeue(self):"},{"line_number":1465,"context_line":"        # This particular test does a large amount of merges and needs a little"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_cbb7c93a","line":1462,"in_reply_to":"df3967d1_60e11cd1","updated":"2017-07-28 20:36:20.000000000","message":"You define autohold for one node. So I think it might make sense to add a second change triggering a second job and ensuring that no further node is held.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5a9ace988081b6de44b63505deb80ae5be13aaed","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"            held_node[\u0027hold_job\u0027],"},{"line_number":1460,"context_line":"            \" \".join([\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027])"},{"line_number":1461,"context_line":"        )"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"    @simple_layout(\u0027layouts/three-projects.yaml\u0027)"},{"line_number":1464,"context_line":"    def test_dependent_behind_dequeue(self):"},{"line_number":1465,"context_line":"        # This particular test does a large amount of merges and needs a little"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_60e11cd1","line":1462,"in_reply_to":"df3967d1_65f48e5c","updated":"2017-07-28 19:56:18.000000000","message":"What second node? Only one is defined.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":4162,"name":"Paul Belanger","email":"pabelanger@redhat.com","username":"pabelanger"},"change_message_id":"67fdee8b3dd0548ef471c459c84f95b07cff4265","unresolved":false,"context_lines":[{"line_number":1438,"context_line":"    def test_autohold(self):"},{"line_number":1439,"context_line":"        A \u003d self.fake_gerrit.addFakeChange(\u0027org/project\u0027, \u0027master\u0027, \u0027A\u0027)"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"        self.sched.autohold(\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027, 1)"},{"line_number":1442,"context_line":""},{"line_number":1443,"context_line":"        self.executor_server.failJob(\u0027project-test2\u0027, A)"},{"line_number":1444,"context_line":"        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))"}],"source_content_type":"text/x-python","patch_set":9,"id":"df3967d1_814cd8c7","line":1441,"updated":"2017-07-31 18:03:48.000000000","message":"Do you think we can update this to use rpcclient.autohold? This would give more coverage like we did for client_enqueue tests.","commit_id":"4fae0413c6bd48621f27251629fd921e7d8f8bcb"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"e7a9a4c217315f0df7ce50535ca69f1f167ddd4c","unresolved":false,"context_lines":[{"line_number":1438,"context_line":"    def test_autohold(self):"},{"line_number":1439,"context_line":"        A \u003d self.fake_gerrit.addFakeChange(\u0027org/project\u0027, \u0027master\u0027, \u0027A\u0027)"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"        self.sched.autohold(\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027, 1)"},{"line_number":1442,"context_line":""},{"line_number":1443,"context_line":"        self.executor_server.failJob(\u0027project-test2\u0027, A)"},{"line_number":1444,"context_line":"        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))"}],"source_content_type":"text/x-python","patch_set":9,"id":"df3967d1_c4cd4e5e","line":1441,"in_reply_to":"df3967d1_814cd8c7","updated":"2017-07-31 18:52:06.000000000","message":"Done","commit_id":"4fae0413c6bd48621f27251629fd921e7d8f8bcb"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"babf051ed1703107a86291d1217c2d82d3f8e8a7","unresolved":false,"context_lines":[{"line_number":1463,"context_line":"            held_node[\u0027hold_job\u0027],"},{"line_number":1464,"context_line":"            \" \".join([\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027])"},{"line_number":1465,"context_line":"        )"},{"line_number":1466,"context_line":""},{"line_number":1467,"context_line":"    @simple_layout(\u0027layouts/three-projects.yaml\u0027)"},{"line_number":1468,"context_line":"    def test_dependent_behind_dequeue(self):"},{"line_number":1469,"context_line":"        # This particular test does a large amount of merges and needs a little"}],"source_content_type":"text/x-python","patch_set":9,"id":"df3967d1_e1d044fa","line":1466,"updated":"2017-07-31 18:00:41.000000000","message":"I think the test should continue with a check if count is respected by autohold. This could be done by adding another fake change and counting the held nodes afterwards.","commit_id":"4fae0413c6bd48621f27251629fd921e7d8f8bcb"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"e7a9a4c217315f0df7ce50535ca69f1f167ddd4c","unresolved":false,"context_lines":[{"line_number":1463,"context_line":"            held_node[\u0027hold_job\u0027],"},{"line_number":1464,"context_line":"            \" \".join([\u0027tenant-one\u0027, \u0027org/project\u0027, \u0027project-test2\u0027])"},{"line_number":1465,"context_line":"        )"},{"line_number":1466,"context_line":""},{"line_number":1467,"context_line":"    @simple_layout(\u0027layouts/three-projects.yaml\u0027)"},{"line_number":1468,"context_line":"    def test_dependent_behind_dequeue(self):"},{"line_number":1469,"context_line":"        # This particular test does a large amount of merges and needs a little"}],"source_content_type":"text/x-python","patch_set":9,"id":"df3967d1_c452ee67","line":1466,"in_reply_to":"df3967d1_e1d044fa","updated":"2017-07-31 18:52:06.000000000","message":"Done","commit_id":"4fae0413c6bd48621f27251629fd921e7d8f8bcb"}],"zuul/cmd/client.py":[{"author":{"_account_id":4162,"name":"Paul Belanger","email":"pabelanger@redhat.com","username":"pabelanger"},"change_message_id":"67fdee8b3dd0548ef471c459c84f95b07cff4265","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        else:"},{"line_number":151,"context_line":"            sys.exit(1)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def autohold(self):"},{"line_number":154,"context_line":"        client \u003d zuul.rpcclient.RPCClient("},{"line_number":155,"context_line":"            self.server, self.port, self.ssl_key, self.ssl_cert, self.ssl_ca)"},{"line_number":156,"context_line":"        r \u003d client.autohold(tenant_name\u003dself.args.tenant,"}],"source_content_type":"text/x-python","patch_set":9,"id":"df3967d1_41ccb034","line":153,"updated":"2017-07-31 18:03:48.000000000","message":"-1 Please add documentation to zuul client section.","commit_id":"4fae0413c6bd48621f27251629fd921e7d8f8bcb"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"e7a9a4c217315f0df7ce50535ca69f1f167ddd4c","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        else:"},{"line_number":151,"context_line":"            sys.exit(1)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def autohold(self):"},{"line_number":154,"context_line":"        client \u003d zuul.rpcclient.RPCClient("},{"line_number":155,"context_line":"            self.server, self.port, self.ssl_key, self.ssl_cert, self.ssl_ca)"},{"line_number":156,"context_line":"        r \u003d client.autohold(tenant_name\u003dself.args.tenant,"}],"source_content_type":"text/x-python","patch_set":9,"id":"df3967d1_c4730ee7","line":153,"in_reply_to":"df3967d1_41ccb034","updated":"2017-07-31 18:52:06.000000000","message":"Done","commit_id":"4fae0413c6bd48621f27251629fd921e7d8f8bcb"}],"zuul/rpcclient.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b320e44de3d52625a6c428df0e508a7ba2a90e26","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                \u0027project_name\u0027: project_name,"},{"line_number":54,"context_line":"                \u0027job_name\u0027: job_name,"},{"line_number":55,"context_line":"                \u0027count\u0027: count}"},{"line_number":56,"context_line":"        return not self.submitJob(\u0027zuul:autohold\u0027, data).failure"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def enqueue(self, tenant, pipeline, project, trigger, change):"},{"line_number":59,"context_line":"        data \u003d {\u0027tenant\u0027: tenant,"}],"source_content_type":"text/x-python","patch_set":11,"id":"df3967d1_d094e846","line":56,"updated":"2017-07-31 20:18:35.000000000","message":"This feels a little nitpicky, sorry.  But I think it would be better to put this json structure back the way you had it in PS6 (with no \"_name\" suffixes), which matched the other functions here.  There\u0027s no difference between the \"project_name\" here and the \"project\" in the method below.\n\nFixed in   https://review.openstack.org/489375","commit_id":"ffab07a8446de3ddfd019dd51897948531b329df"}],"zuul/rpclistener.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"9592384131719e969c4d1b8692f04c1d376a515c","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        (trusted, project) \u003d tenant.getProject(args[\u0027project\u0027])"},{"line_number":106,"context_line":"        if project:"},{"line_number":107,"context_line":"            params[\u0027project\u0027] \u003d str(project)"},{"line_number":108,"context_line":"        else:"},{"line_number":109,"context_line":"            error \u003d \"Invalid project: %s\" % args[\u0027project\u0027]"},{"line_number":110,"context_line":"            job.sendWorkException(error.encode(\u0027utf8\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_0524a2c5","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":44},"updated":"2017-07-28 19:37:14.000000000","message":"What representation should it be here? Name, canonocal name or possibly the project object itself?\n\nAs the project name can be ambiguous (even in a tenant) I would at least use the canonical name here.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5a9ace988081b6de44b63505deb80ae5be13aaed","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        (trusted, project) \u003d tenant.getProject(args[\u0027project\u0027])"},{"line_number":106,"context_line":"        if project:"},{"line_number":107,"context_line":"            params[\u0027project\u0027] \u003d str(project)"},{"line_number":108,"context_line":"        else:"},{"line_number":109,"context_line":"            error \u003d \"Invalid project: %s\" % args[\u0027project\u0027]"},{"line_number":110,"context_line":"            job.sendWorkException(error.encode(\u0027utf8\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_40b178e1","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":44},"in_reply_to":"df3967d1_0524a2c5","updated":"2017-07-28 19:56:18.000000000","message":"Shouldn\u0027t be the object, but should be one of the str names. Wasn\u0027t sure which to use, but can certainly switch it to canonical if that\u0027s the better choice.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"255c0de352ceb302d4256cc8a367f59e1b900a98","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        (trusted, project) \u003d tenant.getProject(args[\u0027project\u0027])"},{"line_number":106,"context_line":"        if project:"},{"line_number":107,"context_line":"            params[\u0027project\u0027] \u003d str(project)"},{"line_number":108,"context_line":"        else:"},{"line_number":109,"context_line":"            error \u003d \"Invalid project: %s\" % args[\u0027project\u0027]"},{"line_number":110,"context_line":"            job.sendWorkException(error.encode(\u0027utf8\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_8ba0a1a7","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":44},"in_reply_to":"df3967d1_40b178e1","updated":"2017-07-28 20:36:20.000000000","message":"Yeah, I think the canonical is the more robust here.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"bec4d6a915c9484f0289a13f31de11f9d0087fc5","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        (trusted, project) \u003d tenant.getProject(args[\u0027project\u0027])"},{"line_number":106,"context_line":"        if project:"},{"line_number":107,"context_line":"            params[\u0027project\u0027] \u003d str(project)"},{"line_number":108,"context_line":"        else:"},{"line_number":109,"context_line":"            error \u003d \"Invalid project: %s\" % args[\u0027project\u0027]"},{"line_number":110,"context_line":"            job.sendWorkException(error.encode(\u0027utf8\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_ea670225","line":107,"range":{"start_line":107,"start_character":32,"end_line":107,"end_character":44},"in_reply_to":"df3967d1_8ba0a1a7","updated":"2017-07-31 14:42:18.000000000","message":"Done","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"9592384131719e969c4d1b8692f04c1d376a515c","unresolved":false,"context_lines":[{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        params[\u0027count\u0027] \u003d args[\u0027count\u0027]"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.sched.autohold(**params)"},{"line_number":123,"context_line":"        job.sendWorkComplete()"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def _common_enqueue(self, job):"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_200d5437","line":122,"range":{"start_line":122,"start_character":28,"end_line":122,"end_character":36},"updated":"2017-07-28 19:37:14.000000000","message":"Not sure if it\u0027s a good practice to put a dict into positional args.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5a9ace988081b6de44b63505deb80ae5be13aaed","unresolved":false,"context_lines":[{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        params[\u0027count\u0027] \u003d args[\u0027count\u0027]"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.sched.autohold(**params)"},{"line_number":123,"context_line":"        job.sendWorkComplete()"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def _common_enqueue(self, job):"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_60935c51","line":122,"range":{"start_line":122,"start_character":28,"end_line":122,"end_character":36},"in_reply_to":"df3967d1_200d5437","updated":"2017-07-28 19:56:18.000000000","message":"It will use keyword args, not positional.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"10279d73b31579878b6e6f88525d3e23c378d859","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        params[\u0027job\u0027] \u003d args[\u0027job\u0027]"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        if args[\u0027count\u0027] \u003c 1:"},{"line_number":116,"context_line":"            error \u003d \"Invalid count: %d\" % args[\u0027count\u0027]"},{"line_number":117,"context_line":"            job.sendWorkException(error.encode(\u0027utf8\u0027))"},{"line_number":118,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_0ba2719b","line":115,"updated":"2017-07-28 20:40:13.000000000","message":"0: We might want to allow count to be 0 to indicate \"remove the existing autohold request\".","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"ec8a71082604ff267c0d12e49f69a70366177129","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        params[\u0027job\u0027] \u003d args[\u0027job\u0027]"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        if args[\u0027count\u0027] \u003c 1:"},{"line_number":116,"context_line":"            error \u003d \"Invalid count: %d\" % args[\u0027count\u0027]"},{"line_number":117,"context_line":"            job.sendWorkException(error.encode(\u0027utf8\u0027))"},{"line_number":118,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_0f193878","line":115,"in_reply_to":"df3967d1_0ba2719b","updated":"2017-07-31 14:42:42.000000000","message":"Done","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"}],"zuul/scheduler.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"9592384131719e969c4d1b8692f04c1d376a515c","unresolved":false,"context_lines":[{"line_number":363,"context_line":"        if job not in self.autohold_requests[tenant][project]:"},{"line_number":364,"context_line":"            self.autohold_requests[tenant][project][job] \u003d count"},{"line_number":365,"context_line":"        else:"},{"line_number":366,"context_line":"            raise Exception(\"Autohold already active for job %s\" % job)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def promote(self, tenant_name, pipeline_name, change_ids):"},{"line_number":369,"context_line":"        event \u003d PromoteEvent(tenant_name, pipeline_name, change_ids)"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_20743499","line":366,"range":{"start_line":366,"start_character":12,"end_line":366,"end_character":71},"updated":"2017-07-28 19:37:14.000000000","message":"Maybe one might want to change the count for an already active autohold?","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"255c0de352ceb302d4256cc8a367f59e1b900a98","unresolved":false,"context_lines":[{"line_number":363,"context_line":"        if job not in self.autohold_requests[tenant][project]:"},{"line_number":364,"context_line":"            self.autohold_requests[tenant][project][job] \u003d count"},{"line_number":365,"context_line":"        else:"},{"line_number":366,"context_line":"            raise Exception(\"Autohold already active for job %s\" % job)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def promote(self, tenant_name, pipeline_name, change_ids):"},{"line_number":369,"context_line":"        event \u003d PromoteEvent(tenant_name, pipeline_name, change_ids)"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_4b5999fc","line":366,"range":{"start_line":366,"start_character":12,"end_line":366,"end_character":71},"in_reply_to":"df3967d1_00f9307f","updated":"2017-07-28 20:36:20.000000000","message":"Don\u0027t know if we want that. But I can imagine the following scenario:\n\n  Dev: Can you hold me a node for job foo?\n  Ops: sure... types \u0027zuul hold openstack foo test-foo 100\u0027\n  Ops: Shit I didn\u0027t want to hold 100 nodes...\n  Ops: types \u0027zuul hold openstack foo test-foo 1\u0027\n  Zuul: Wuahaha, autohold is already active\n  Ops: :( Now I have to delete 100 held nodes by hand","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5a9ace988081b6de44b63505deb80ae5be13aaed","unresolved":false,"context_lines":[{"line_number":363,"context_line":"        if job not in self.autohold_requests[tenant][project]:"},{"line_number":364,"context_line":"            self.autohold_requests[tenant][project][job] \u003d count"},{"line_number":365,"context_line":"        else:"},{"line_number":366,"context_line":"            raise Exception(\"Autohold already active for job %s\" % job)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def promote(self, tenant_name, pipeline_name, change_ids):"},{"line_number":369,"context_line":"        event \u003d PromoteEvent(tenant_name, pipeline_name, change_ids)"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_00f9307f","line":366,"range":{"start_line":366,"start_character":12,"end_line":366,"end_character":71},"in_reply_to":"df3967d1_20743499","updated":"2017-07-28 19:56:18.000000000","message":"Maybe? Is that something we want?","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"10279d73b31579878b6e6f88525d3e23c378d859","unresolved":false,"context_lines":[{"line_number":363,"context_line":"        if job not in self.autohold_requests[tenant][project]:"},{"line_number":364,"context_line":"            self.autohold_requests[tenant][project][job] \u003d count"},{"line_number":365,"context_line":"        else:"},{"line_number":366,"context_line":"            raise Exception(\"Autohold already active for job %s\" % job)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def promote(self, tenant_name, pipeline_name, change_ids):"},{"line_number":369,"context_line":"        event \u003d PromoteEvent(tenant_name, pipeline_name, change_ids)"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_eb9e0d53","line":366,"updated":"2017-07-28 20:40:13.000000000","message":"0: We might want to allow updating the request, both in terms of removing it with a zero count, or simply changing the number.  If we do, this makes your paranoia below rather forward thinking.  :)","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"ec8a71082604ff267c0d12e49f69a70366177129","unresolved":false,"context_lines":[{"line_number":363,"context_line":"        if job not in self.autohold_requests[tenant][project]:"},{"line_number":364,"context_line":"            self.autohold_requests[tenant][project][job] \u003d count"},{"line_number":365,"context_line":"        else:"},{"line_number":366,"context_line":"            raise Exception(\"Autohold already active for job %s\" % job)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def promote(self, tenant_name, pipeline_name, change_ids):"},{"line_number":369,"context_line":"        event \u003d PromoteEvent(tenant_name, pipeline_name, change_ids)"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_4f3d00ba","line":366,"in_reply_to":"df3967d1_eb9e0d53","updated":"2017-07-31 14:42:42.000000000","message":"Done","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"10279d73b31579878b6e6f88525d3e23c378d859","unresolved":false,"context_lines":[{"line_number":840,"context_line":"        if tenant in self.autohold_requests:"},{"line_number":841,"context_line":"            if project in self.autohold_requests[tenant]:"},{"line_number":842,"context_line":"                if job in self.autohold_requests[tenant][project]:"},{"line_number":843,"context_line":"                    return self.autohold_requests[tenant][project][job]"},{"line_number":844,"context_line":"        return 0"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def _removeAutohold(self, tenant, project, job):"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_cbab4973","line":843,"updated":"2017-07-28 20:40:13.000000000","message":"nit: Sometimes for things like this where the intermediate data structures don\u0027t matter, I just let python hash a tuple.  Like:\n\nkey \u003d (tenant, project, job)\nself.autohold_requests[key] \u003d foo","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"ec8a71082604ff267c0d12e49f69a70366177129","unresolved":false,"context_lines":[{"line_number":840,"context_line":"        if tenant in self.autohold_requests:"},{"line_number":841,"context_line":"            if project in self.autohold_requests[tenant]:"},{"line_number":842,"context_line":"                if job in self.autohold_requests[tenant][project]:"},{"line_number":843,"context_line":"                    return self.autohold_requests[tenant][project][job]"},{"line_number":844,"context_line":"        return 0"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def _removeAutohold(self, tenant, project, job):"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_cfed7026","line":843,"in_reply_to":"df3967d1_cbab4973","updated":"2017-07-31 14:42:42.000000000","message":"Done","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"10279d73b31579878b6e6f88525d3e23c378d859","unresolved":false,"context_lines":[{"line_number":857,"context_line":"        # to pass this on to the pipeline manager, make sure we return"},{"line_number":858,"context_line":"        # the nodes to nodepool."},{"line_number":859,"context_line":"        try:"},{"line_number":860,"context_line":"            nodeset \u003d build.build_set.getJobNodeSet(build.job.name)"},{"line_number":861,"context_line":"            tenant \u003d build.pipeline.layout.tenant.name"},{"line_number":862,"context_line":"            project \u003d build.build_set.item.change.project.name"},{"line_number":863,"context_line":"            job \u003d build.job.name"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_aba80577","line":860,"updated":"2017-07-28 20:40:13.000000000","message":"-1ish: We should probably put all the new code in its own try/except handler so if there\u0027s an error with holding the node, we log it, but then continue to attempt to return the nodeset.\n\nThat might be a bit easier to accomplish if you move most of the code into one or two methods on zuul.Nodepool (eg, holdNodSet() could do the work and return the count, or make holdNodeSet() and countHeldNodes()).","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"ec8a71082604ff267c0d12e49f69a70366177129","unresolved":false,"context_lines":[{"line_number":857,"context_line":"        # to pass this on to the pipeline manager, make sure we return"},{"line_number":858,"context_line":"        # the nodes to nodepool."},{"line_number":859,"context_line":"        try:"},{"line_number":860,"context_line":"            nodeset \u003d build.build_set.getJobNodeSet(build.job.name)"},{"line_number":861,"context_line":"            tenant \u003d build.pipeline.layout.tenant.name"},{"line_number":862,"context_line":"            project \u003d build.build_set.item.change.project.name"},{"line_number":863,"context_line":"            job \u003d build.job.name"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_ca7d5e6f","line":860,"in_reply_to":"df3967d1_aba80577","updated":"2017-07-31 14:42:42.000000000","message":"Done","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"}],"zuul/zk.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"9592384131719e969c4d1b8692f04c1d376a515c","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        node.lock.release()"},{"line_number":252,"context_line":"        node.lock \u003d None"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def heldNodeCount(self, tenant, project, job):"},{"line_number":255,"context_line":"        \u0027\u0027\u0027"},{"line_number":256,"context_line":"        Count the number of nodes being held for the given tenant/project/job."},{"line_number":257,"context_line":"        \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_20f794f8","line":254,"range":{"start_line":254,"start_character":36,"end_line":254,"end_character":43},"updated":"2017-07-28 19:37:14.000000000","message":"For me the project argument is confusing. Is this the project object, project name or canonical project name?\n\nThis could be annotated or made clearer with naming.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"5a9ace988081b6de44b63505deb80ae5be13aaed","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        node.lock.release()"},{"line_number":252,"context_line":"        node.lock \u003d None"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def heldNodeCount(self, tenant, project, job):"},{"line_number":255,"context_line":"        \u0027\u0027\u0027"},{"line_number":256,"context_line":"        Count the number of nodes being held for the given tenant/project/job."},{"line_number":257,"context_line":"        \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_a0ef04af","line":254,"range":{"start_line":254,"start_character":36,"end_line":254,"end_character":43},"in_reply_to":"df3967d1_20f794f8","updated":"2017-07-28 19:56:18.000000000","message":"They should all be str\u0027s for the names, but I can certainly add _name to each one if that\u0027s clearer (or add docstrings, or both).","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"255c0de352ceb302d4256cc8a367f59e1b900a98","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        node.lock.release()"},{"line_number":252,"context_line":"        node.lock \u003d None"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def heldNodeCount(self, tenant, project, job):"},{"line_number":255,"context_line":"        \u0027\u0027\u0027"},{"line_number":256,"context_line":"        Count the number of nodes being held for the given tenant/project/job."},{"line_number":257,"context_line":"        \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_6bd69d52","line":254,"range":{"start_line":254,"start_character":36,"end_line":254,"end_character":43},"in_reply_to":"df3967d1_a0ef04af","updated":"2017-07-28 20:36:20.000000000","message":"That would be cool. We currently have an inconsistent usage of project/project_name, tenant/tenant_name, etc at several locations where I think we need to cleanup a bit in the long run.","commit_id":"8482a243eb1367c915637f814dde7b8ff4548fa5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"10279d73b31579878b6e6f88525d3e23c378d859","unresolved":false,"context_lines":[{"line_number":268,"context_line":"            node_data \u003d self._strToDict(node_data)"},{"line_number":269,"context_line":"            if (node_data[\u0027state\u0027] \u003d\u003d zuul.model.STATE_HOLD and"},{"line_number":270,"context_line":"                    node_data.get(\u0027hold_job\u0027) and"},{"line_number":271,"context_line":"                    node_data[\u0027hold_job\u0027] \u003d\u003d identifier):"},{"line_number":272,"context_line":"                count +\u003d 1"},{"line_number":273,"context_line":"        return count"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_8bb781d9","line":271,"updated":"2017-07-28 20:40:13.000000000","message":"nit: shorter as: node_data.get(\u0027hold_job\u0027) \u003d\u003d identifier\n(since we know identifier is not None)","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"ec8a71082604ff267c0d12e49f69a70366177129","unresolved":false,"context_lines":[{"line_number":268,"context_line":"            node_data \u003d self._strToDict(node_data)"},{"line_number":269,"context_line":"            if (node_data[\u0027state\u0027] \u003d\u003d zuul.model.STATE_HOLD and"},{"line_number":270,"context_line":"                    node_data.get(\u0027hold_job\u0027) and"},{"line_number":271,"context_line":"                    node_data[\u0027hold_job\u0027] \u003d\u003d identifier):"},{"line_number":272,"context_line":"                count +\u003d 1"},{"line_number":273,"context_line":"        return count"}],"source_content_type":"text/x-python","patch_set":6,"id":"df3967d1_caf2fe06","line":271,"in_reply_to":"df3967d1_8bb781d9","updated":"2017-07-31 14:42:42.000000000","message":"Done","commit_id":"79ddb61ce3eb8a65607e24b301963208f7cfb7a4"}]}