		`@@ -1069,6 +1069,8 @@`
		`chroots = filter(lambda x: x.status != StatusEnum("succeeded"), build.build_chroots)`
		`for chroot in chroots:`
		`chroot.status = StatusEnum("failed")`
		`+ if build.source_status != StatusEnum("succeeded"):`
		`+ build.source_status = StatusEnum("failed")`
		`cls.process_update_callback(build)`
		`return build`

		`@@ -951,7 +951,7 @@`

		`@property`
		`def chroot_states(self):`
		`- return map(lambda chroot: chroot.status, self.build_chroots)`
		`+ return list(map(lambda chroot: chroot.status, self.build_chroots))`

		`def get_chroots_by_status(self, statuses=None):`
		`"""`
		`@@ -981,16 +981,34 @@`
		`if self.canceled:`
		`return StatusEnum("canceled")`

		`- use_src_statuses = ["starting", "pending", "running", "failed"]`
		`+ use_src_statuses = ["starting", "pending", "running", "importing", "failed"]`
		`if self.source_status in [StatusEnum(s) for s in use_src_statuses]:`
		`return self.source_status`

		`- for state in ["running", "starting", "pending", "failed", "succeeded", "skipped", "forked", "waiting"]:`
		`+ if not self.chroot_states:`
		`+ # There were some builds in DB which had source_status equal`
		`+ # to 'succeeded', while they had no biuld_chroots created.`
		`+ # The original source of this inconsistency isn't known`
		`+ # because we only ever flip source_status to "succeded" directly`
		`+ # from the "importing" state.`
		`+ # Anyways, return something meaningful here so we can debug`
		`+ # properly if such situation happens.`
		`+ app.logger.error("Build %s has source_status succeeded, but "`
		`+ "no build_chroots", self.id)`
		`+ return StatusEnum("waiting")`
		`+`
		`+ for state in ["running", "starting", "pending", "failed", "succeeded", "skipped", "forked"]:`
		`if StatusEnum(state) in self.chroot_states:`
		`- if state == "waiting":`
		`- return self.source_status`
		`- else:`
		`- return StatusEnum(state)`
		`+ return StatusEnum(state)`
		`+`
		`+ if StatusEnum("waiting") in self.chroot_states:`
		`+ # We should atomically flip`
		`+ # a) build.source_status: "importing" -> "succeeded" and`
		`+ # b) biuld_chroot.status: "waiting" -> "pending"`
		`+ # so at this point nothing really should be in "waiting" state.`
		`+ app.logger.error("Build chroots pending, even though build %s"`
		`+ " has succeeded source_status", self.id)`
		`+ return StatusEnum("pending")`

		`return None`

		`@@ -165,11 +165,7 @@`
		`{{ build_state_text("canceled") }}`
		`{% else %}`
		`{% if build.status is not none %}`
		`- {% if build.status\|state_from_num == "waiting" %}`
		`- {{ build_state_text(build.source_status\|state_from_num) }}`
		`- {% else %}`
		`- {{ build_state_text(build.status\|state_from_num) }}`
		`- {% endif %}`
		`+ {{ build_state_text(build.status\|state_from_num) }}`
		`{% elif build.source_status %}`
		`{{ build_state_text(build.source_status\|state_from_num) }}`
		`{% else %}`

		`@@ -236,9 +236,14 @@`
		`copr=self.c2, copr_dir=self.c3_dir, name="goodbye-world", source_type=0)`

		`self.b1 = models.Build(`
		`- copr=self.c1, copr_dir=self.c1_dir, package=self.p1, user=self.u1, submitted_on=50, srpm_url="http://somesrpm", source_status=StatusEnum("importing"), result_dir='bar')`
		`+ copr=self.c1, copr_dir=self.c1_dir, package=self.p1,`
		`+ user=self.u1, submitted_on=50, srpm_url="http://somesrpm",`
		`+ source_status=StatusEnum("succeeded"), result_dir='bar')`
		`self.b2 = models.Build(`
		`- copr=self.c1, copr_dir=self.c1_dir, package=self.p1, user=self.u2, submitted_on=10, srpm_url="http://somesrpm", source_status=StatusEnum("importing"), result_dir='bar')`
		`+ copr=self.c1, copr_dir=self.c1_dir, package=self.p1,`
		`+ user=self.u2, submitted_on=10, srpm_url="http://somesrpm",`
		`+ source_status=StatusEnum("importing"), result_dir='bar',`
		`+ source_json='{}')`
		`self.b3 = models.Build(`
		`copr=self.c2, copr_dir=self.c2_dir, package=self.p2, user=self.u2, submitted_on=10, srpm_url="http://somesrpm", source_status=StatusEnum("importing"), result_dir='bar')`
		`self.b4 = models.Build(`

		`@@ -96,7 +96,7 @@`
		`def test_build_queue_1(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):`
		`self.db.session.commit()`
		`data = BuildsLogic.get_build_importing_queue().all()`
		`- assert len(data) == 3`
		`+ assert len(data) == 2`

		`def test_build_queue_2(self, f_users, f_coprs, f_mock_chroots, f_db):`
		`self.db.session.commit()`
		`@@ -226,7 +226,9 @@`
		`with pytest.raises(NoResultFound):`
		`BuildsLogic.get(self.b1.id).one()`

		`- def test_mark_as_failed(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db):`
		`+ def test_mark_as_failed(self, f_users, f_coprs, f_mock_chroots, f_builds):`
		`+ self.b1.source_status = StatusEnum("succeeded")`
		`+ self.db.session.commit()`
		`BuildsLogic.mark_as_failed(self.b1.id)`
		`BuildsLogic.mark_as_failed(self.b3.id)`

		`@@ -130,6 +130,16 @@`
		`]`
		`}"""`

		`+ import_data1 = """`
		`+ {`
		`+ "build_id": 2,`
		`+ "branch_commits": {`
		`+ "f28": "4dc32823233c0ef1aacc6f345b674d4f40a026b8"`
		`+ },`
		`+ "reponame": "test/foo"`
		`+ }`
		`+ """`
		`+`
		`def test_updating_requires_password(self, f_users, f_coprs, f_builds, f_db):`
		`r = self.tc.post("/backend/update/",`
		`content_type="application/json",`
		`@@ -162,6 +172,13 @@`
		`self.db.session.add_all([self.b1, self.b2])`
		`self.db.session.commit()`

		`+ # test that import hook works`
		`+ r = self.tc.post("/backend/import-completed/",`
		`+ content_type="application/json",`
		`+ headers=self.auth_header,`
		`+ data=self.import_data1)`
		`+ assert r.status_code == 200`
		`+`
		`r = self.tc.post("/backend/update/",`
		`content_type="application/json",`
		`headers=self.auth_header,`

copr / copr

Source Code

Documentation

#826 Fix for inconsistency in build/chroot statuses Merged 4 years ago by frostyx. Opened 4 years ago by praiskup. Unknown source build-status-once-more into master

Metadata

Flags

Changes Summary 6

#826 Fix for inconsistency in build/chroot statuses

Merged 4 years ago by frostyx. Opened 4 years ago by praiskup.

Unknown source build-status-once-more into master