)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"101075618d6adff4f585e5908de10f1a2193d1a3","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When a job is retried, the variables set by the zuul_return are not"},{"line_number":10,"context_line":"overwritten."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ia78eac82a978740766ef5a3f5e5e32a02a8d4479"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3f1d41d2_fa1e8e64","line":11,"updated":"2020-12-11 08:10:48.000000000","message":"thanks for working on that. fwiw we are hitting this issue with tripleo upgrade jobs @ https://bugs.launchpad.net/tripleo/+bug/1907657","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"}],"tests/unit/test_v3.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"0e2f8c208f871628e2124f13622dea2362b41009","unresolved":false,"context_lines":[{"line_number":3779,"context_line":"        self.executor_server.release(\u0027paused-data-return-vars\u0027)"},{"line_number":3780,"context_line":"        self.waitUntilSettled(\"till job is paused\")"},{"line_number":3781,"context_line":""},{"line_number":3782,"context_line":"        for _ in iterate_timeout(60, \u0027waiting for paused job\u0027):"},{"line_number":3783,"context_line":"            if len(self.builds) \u003d\u003d 2:"},{"line_number":3784,"context_line":"                break"},{"line_number":3785,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_0c052d14","line":3782,"updated":"2020-03-05 12:21:21.000000000","message":"nit: maybe put a comment here explain why we need to iterate_timeout here and the waitUntilSettled() is not enough.","commit_id":"75cd31b304a7d391ed04bded8066b5195c223a75"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"0e2f8c208f871628e2124f13622dea2362b41009","unresolved":false,"context_lines":[{"line_number":3798,"context_line":""},{"line_number":3799,"context_line":"        self.executor_server.hold_jobs_in_build \u003d False"},{"line_number":3800,"context_line":"        self.executor_server.release(\u0027print-data-return-vars\u0027)"},{"line_number":3801,"context_line":"        self.waitUntilSettled(\"all jobs are done\")"},{"line_number":3802,"context_line":""},{"line_number":3803,"context_line":"        # First build of paused job (gets retired)"},{"line_number":3804,"context_line":"        first_build \u003d self.history[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_4f3d57c2","line":3801,"updated":"2020-03-05 12:21:21.000000000","message":"how about a iterate_timeout here? From other tests with paused jobs it looks like there should be something like:\n\n        for x in iterate_timeout(60, \u0027paused job\u0027):\n            if not self.builds:\n                break","commit_id":"75cd31b304a7d391ed04bded8066b5195c223a75"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"0e2f8c208f871628e2124f13622dea2362b41009","unresolved":false,"context_lines":[{"line_number":3800,"context_line":"        self.executor_server.release(\u0027print-data-return-vars\u0027)"},{"line_number":3801,"context_line":"        self.waitUntilSettled(\"all jobs are done\")"},{"line_number":3802,"context_line":""},{"line_number":3803,"context_line":"        # First build of paused job (gets retired)"},{"line_number":3804,"context_line":"        first_build \u003d self.history[0]"},{"line_number":3805,"context_line":"        # Second build of the paused job (the retried one)"},{"line_number":3806,"context_line":"        retried_build \u003d self.history[3]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fa4df85_ec52113b","line":3803,"range":{"start_line":3803,"start_character":42,"end_line":3803,"end_character":49},"updated":"2020-03-05 12:21:21.000000000","message":"nit: typo \"retried\"","commit_id":"75cd31b304a7d391ed04bded8066b5195c223a75"}],"zuul/model.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"922b0b2b4d6546cc5e185d290e011c75430a709b","unresolved":false,"context_lines":[{"line_number":1460,"context_line":"        if \u0027zuul\u0027 in v:"},{"line_number":1461,"context_line":"            del v[\u0027zuul\u0027]"},{"line_number":1462,"context_line":"        self.parent_data \u003d v"},{"line_number":1463,"context_line":"        self.variables \u003d Job._deepUpdate(self.variables, self.parent_data)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"        artifact_data \u003d self.artifact_data or []"},{"line_number":1466,"context_line":"        artifacts \u003d get_artifacts_from_result_data(other_vars)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_5fc0ce2a","line":1463,"updated":"2020-03-03 13:51:19.000000000","message":"I think this completely changes the logic of this method (as documented above) and will probably break other use cases.\n\nHow about not mixing self.variables and self.parent_data INTO self.variables, but keeping them separate?\n\nThe problem as I see it is, that we are accumulating all the parent data of previous runs.","commit_id":"600e45b359e74319f38287d812f860c31d50ab13"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"5dccc9feeb8090e9a58912a7745adf009ab43afe","unresolved":false,"context_lines":[{"line_number":2542,"context_line":""},{"line_number":2543,"context_line":"            if all_parent_jobs_successful:"},{"line_number":2544,"context_line":"                # Iterate over all jobs of the graph (which is in sorted config"},{"line_number":2545,"context_line":"                # order) and apply parent data of the jobs we already found."},{"line_number":2546,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2547,"context_line":"                    for parent_job in self.job_graph.getJobs():"},{"line_number":2548,"context_line":"                        parent_build \u003d parent_builds_with_data.get("}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_77d35c61","line":2545,"updated":"2020-03-04 14:47:05.000000000","message":"Why do we change the order here? If I understand the comment correctly, the `reverse()`was on purpose.","commit_id":"09e123ceab8ad11216fc6f7a2c843a55724a95e6"},{"author":{"_account_id":28088,"name":"Benedikt Löffler","email":"benedikt.loeffler@bmw.de","username":"bloeffler"},"change_message_id":"d5dedc13058f5f10e63d06185b91eb4fff469a60","unresolved":false,"context_lines":[{"line_number":2542,"context_line":""},{"line_number":2543,"context_line":"            if all_parent_jobs_successful:"},{"line_number":2544,"context_line":"                # Iterate over all jobs of the graph (which is in sorted config"},{"line_number":2545,"context_line":"                # order) and apply parent data of the jobs we already found."},{"line_number":2546,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2547,"context_line":"                    for parent_job in self.job_graph.getJobs():"},{"line_number":2548,"context_line":"                        parent_build \u003d parent_builds_with_data.get("}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_f7d18cfc","line":2545,"in_reply_to":"1fa4df85_77d35c61","updated":"2020-03-04 15:07:17.000000000","message":"The problem is, if we have 2 jobs A and B and we reverse the order, then the return values of B (which run later) are overwritten with those of A.\n\nPreviously, the self.variables were never updated, only merged.","commit_id":"09e123ceab8ad11216fc6f7a2c843a55724a95e6"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"595db306188995331a38f2a4b282e52b93f6a497","unresolved":false,"context_lines":[{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"    def updateParentData(self, other_build):"},{"line_number":1456,"context_line":"        other_vars \u003d other_build.result_data"},{"line_number":1457,"context_line":"        v \u003d self.parent_data or {}"},{"line_number":1458,"context_line":"        v \u003d Job._deepUpdate(v, other_vars)"},{"line_number":1459,"context_line":"        # To avoid running afoul of checks that jobs don\u0027t set zuul"},{"line_number":1460,"context_line":"        # variables, remove them from parent data here."}],"source_content_type":"text/x-python","patch_set":3,"id":"1fa4df85_2a7b2e7b","line":1457,"updated":"2020-03-05 08:11:48.000000000","message":"I think because of this line here we are still keeping stale data around (from previous call to updateParentData()). If I\u0027m not mistaken we can always re-construct the parent data from the parent builds. So I see no need to mix in the previous parent_data. This way you should also be able can to keep the below reverse() logic.","commit_id":"5ad60342583a64b4724e7acc66d85c51a271ba33"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"585db51ac790475b52d193a697649af531a51eca","unresolved":false,"context_lines":[{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"    def updateParentData(self, other_build):"},{"line_number":1456,"context_line":"        other_vars \u003d other_build.result_data"},{"line_number":1457,"context_line":"        v \u003d self.parent_data or {}"},{"line_number":1458,"context_line":"        v \u003d Job._deepUpdate(v, other_vars)"},{"line_number":1459,"context_line":"        # To avoid running afoul of checks that jobs don\u0027t set zuul"},{"line_number":1460,"context_line":"        # variables, remove them from parent data here."}],"source_content_type":"text/x-python","patch_set":3,"id":"1fa4df85_0ace7281","line":1457,"in_reply_to":"1fa4df85_2a7b2e7b","updated":"2020-03-05 08:49:28.000000000","message":"Let me rephrase that a bit: before calling updateParentData() for all parent jobs we need to (re-)init/clear job.parent_data in findJobsToRun().","commit_id":"5ad60342583a64b4724e7acc66d85c51a271ba33"},{"author":{"_account_id":24162,"name":"Sorin Sbârnea","display_name":"zbr","email":"ssbarnea@redhat.com","username":"ssbarnea","status":"do not feed the troll 🥕"},"change_message_id":"968692a689f1a7cd47cbaf65ce00b134ee3c2fcc","unresolved":true,"context_lines":[{"line_number":1232,"context_line":"        self.name \u003d name"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"    @property"},{"line_number":1235,"context_line":"    def combined_variables(self):"},{"line_number":1236,"context_line":"        return Job._deepUpdate(self.parent_data or {}, self.variables)"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    def toDict(self, tenant):"}],"source_content_type":"text/x-python","patch_set":6,"id":"e18232d1_906b1c2c","line":1235,"range":{"start_line":1235,"start_character":4,"end_line":1235,"end_character":33},"updated":"2020-12-10 16:00:38.000000000","message":"Adding a docstring to this new method would not really hurt. In fact you could also add return type hints unless this does not trigger a very long chain of changes.","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"08f379dd42b4ad49bf8a35419da2b5bd8a8e1678","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"        self.name \u003d name"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"    @property"},{"line_number":1235,"context_line":"    def combined_variables(self):"},{"line_number":1236,"context_line":"        return Job._deepUpdate(self.parent_data or {}, self.variables)"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    def toDict(self, tenant):"}],"source_content_type":"text/x-python","patch_set":6,"id":"282b3616_856e9d5b","line":1235,"updated":"2020-12-10 16:03:09.000000000","message":"We should put a comment here saying \"Job variables have priority over parent data.\"","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":24162,"name":"Sorin Sbârnea","display_name":"zbr","email":"ssbarnea@redhat.com","username":"ssbarnea","status":"do not feed the troll 🥕"},"change_message_id":"63c4b0ea26a31f1c9c9dfd500a660edc28e3cf41","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"        self.name \u003d name"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"    @property"},{"line_number":1235,"context_line":"    def combined_variables(self):"},{"line_number":1236,"context_line":"        return Job._deepUpdate(self.parent_data or {}, self.variables)"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    def toDict(self, tenant):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ebe71b6f_f7d3092e","line":1235,"in_reply_to":"282b3616_856e9d5b","updated":"2020-12-15 12:22:58.000000000","message":"Done","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":24162,"name":"Sorin Sbârnea","display_name":"zbr","email":"ssbarnea@redhat.com","username":"ssbarnea","status":"do not feed the troll 🥕"},"change_message_id":"63c4b0ea26a31f1c9c9dfd500a660edc28e3cf41","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"        self.name \u003d name"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"    @property"},{"line_number":1235,"context_line":"    def combined_variables(self):"},{"line_number":1236,"context_line":"        return Job._deepUpdate(self.parent_data or {}, self.variables)"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    def toDict(self, tenant):"}],"source_content_type":"text/x-python","patch_set":6,"id":"be8f5b5b_58db76bb","line":1235,"range":{"start_line":1235,"start_character":4,"end_line":1235,"end_character":33},"in_reply_to":"e18232d1_906b1c2c","updated":"2020-12-15 12:22:58.000000000","message":"Done","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"08f379dd42b4ad49bf8a35419da2b5bd8a8e1678","unresolved":false,"context_lines":[{"line_number":1455,"context_line":"    def updateParentData(self, other_build):"},{"line_number":1456,"context_line":"        # Update variables, but give the current values priority (used"},{"line_number":1457,"context_line":"        # for job return data which is lower precedence than defined"},{"line_number":1458,"context_line":"        # job vars)."},{"line_number":1459,"context_line":"        other_vars \u003d other_build.result_data"},{"line_number":1460,"context_line":"        v \u003d self.parent_data or {}"},{"line_number":1461,"context_line":"        v \u003d Job._deepUpdate(other_vars, v)"}],"source_content_type":"text/x-python","patch_set":6,"id":"eb1192ad_4e7a1c68","line":1458,"updated":"2020-12-10 16:03:09.000000000","message":"This comment doesn\u0027t makes sense here anymore.  Below, we\u0027re actually giving the *new* parent data priority over the old parent data.  We do still give the job vars priority over all parent data, but that now happens in combined_variables.\n\nWe should just say \"Give new parent vars priority over old in case the job was retried and new, updated data are returned.\"","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"08f379dd42b4ad49bf8a35419da2b5bd8a8e1678","unresolved":false,"context_lines":[{"line_number":1458,"context_line":"        # job vars)."},{"line_number":1459,"context_line":"        other_vars \u003d other_build.result_data"},{"line_number":1460,"context_line":"        v \u003d self.parent_data or {}"},{"line_number":1461,"context_line":"        v \u003d Job._deepUpdate(other_vars, v)"},{"line_number":1462,"context_line":"        # To avoid running afoul of checks that jobs don\u0027t set zuul"},{"line_number":1463,"context_line":"        # variables, remove them from parent data here."},{"line_number":1464,"context_line":"        if \u0027zuul\u0027 in v:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff522c46_f658ab59","line":1461,"updated":"2020-12-10 16:03:09.000000000","message":"It looks like the order of args here is changed so that a retried build will have priority and update the vars.  However, there\u0027s another change that seems like it would zero out the parent_data every time.  Do we need both?","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"08f379dd42b4ad49bf8a35419da2b5bd8a8e1678","unresolved":false,"context_lines":[{"line_number":2548,"context_line":"                # in sorted config order) and apply parent data of the jobs we"},{"line_number":2549,"context_line":"                # already found."},{"line_number":2550,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2551,"context_line":"                    job.parent_data \u003d {}"},{"line_number":2552,"context_line":"                    for parent_job in reversed(self.job_graph.getJobs()):"},{"line_number":2553,"context_line":"                        parent_build \u003d parent_builds_with_data.get("},{"line_number":2554,"context_line":"                            parent_job.name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b6c0a25c_d0d55fad","line":2551,"updated":"2020-12-10 16:03:09.000000000","message":"Why is this necessary if we have reversed the order of arguments when we update the data?","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"3a3c79dead64723c6c7da8cd2f47134460f41d6b","unresolved":false,"context_lines":[{"line_number":2548,"context_line":"                # in sorted config order) and apply parent data of the jobs we"},{"line_number":2549,"context_line":"                # already found."},{"line_number":2550,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2551,"context_line":"                    job.parent_data \u003d {}"},{"line_number":2552,"context_line":"                    for parent_job in reversed(self.job_graph.getJobs()):"},{"line_number":2553,"context_line":"                        parent_build \u003d parent_builds_with_data.get("},{"line_number":2554,"context_line":"                            parent_job.name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"eab63583_114102ed","line":2551,"in_reply_to":"b6c0a25c_d0d55fad","updated":"2020-12-14 12:10:05.000000000","message":"I\u0027ll check of zeroing out the parent data is enough.","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"8f53d7d5f5a22fb7565194b3a33731327630f6df","unresolved":false,"context_lines":[{"line_number":2548,"context_line":"                # in sorted config order) and apply parent data of the jobs we"},{"line_number":2549,"context_line":"                # already found."},{"line_number":2550,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2551,"context_line":"                    job.parent_data \u003d {}"},{"line_number":2552,"context_line":"                    for parent_job in reversed(self.job_graph.getJobs()):"},{"line_number":2553,"context_line":"                        parent_build \u003d parent_builds_with_data.get("},{"line_number":2554,"context_line":"                            parent_job.name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9b3f5bdd_54162b26","line":2551,"in_reply_to":"eab63583_114102ed","updated":"2020-12-14 12:38:56.000000000","message":"Zeroing out the parent data is enough to fix. So I reverted this part.","commit_id":"b51464d24cbfa005abb49475bd49be97f5f31103"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"c32182696a3f381a76954412086aafdbc57edc41","unresolved":true,"context_lines":[{"line_number":2552,"context_line":"                # in sorted config order) and apply parent data of the jobs we"},{"line_number":2553,"context_line":"                # already found."},{"line_number":2554,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2555,"context_line":"                    job.parent_data \u003d {}"},{"line_number":2556,"context_line":"                    for parent_job in reversed(self.job_graph.getJobs()):"},{"line_number":2557,"context_line":"                        parent_build \u003d parent_builds_with_data.get("},{"line_number":2558,"context_line":"                            parent_job.name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"acb34b43_1aaab7d8","line":2555,"range":{"start_line":2555,"start_character":20,"end_line":2555,"end_character":40},"updated":"2020-12-14 14:04:21.000000000","message":"isn\u0027t this removing all returned variable from previous parents (not just the conflicting one)?","commit_id":"25611f1415d83f5baed918945ad66dc842f8ab73"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"5fadb705c46f8403c7a1e962e0a5efed7b88700c","unresolved":false,"context_lines":[{"line_number":2552,"context_line":"                # in sorted config order) and apply parent data of the jobs we"},{"line_number":2553,"context_line":"                # already found."},{"line_number":2554,"context_line":"                if len(parent_builds_with_data) \u003e 0:"},{"line_number":2555,"context_line":"                    job.parent_data \u003d {}"},{"line_number":2556,"context_line":"                    for parent_job in reversed(self.job_graph.getJobs()):"},{"line_number":2557,"context_line":"                        parent_build \u003d parent_builds_with_data.get("},{"line_number":2558,"context_line":"                            parent_job.name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3e14a75b_52ab2f27","line":2555,"range":{"start_line":2555,"start_character":20,"end_line":2555,"end_character":40},"in_reply_to":"acb34b43_1aaab7d8","updated":"2020-12-14 15:25:18.000000000","message":"oops i missed the nested loop below.","commit_id":"25611f1415d83f5baed918945ad66dc842f8ab73"}]}