Guruprasad has proposed merging ~lgp171188/launchpad:disallow-null-values-bug-lock-status into launchpad:master with ~lgp171188/launchpad:lock-status-not-nullable as a prerequisite.
Commit message: Disallow None as a value for Bug.lock_status Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/415261 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:disallow-null-values-bug-lock-status into launchpad:master.
diff --git a/database/schema/patch-2210-38-1.sql b/database/schema/patch-2210-38-1.sql deleted file mode 100644 index 6519b25..0000000 --- a/database/schema/patch-2210-38-1.sql +++ /dev/null @@ -1,9 +0,0 @@ --- Copyright 2022 Canonical Ltd. This software is licensed under the --- GNU Affero General Public License version 3 (see the file LICENSE). - -SET client_min_messages=ERROR; - -ALTER TABLE Bug - ALTER COLUMN lock_status SET NOT NULL; - -INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 38, 1); diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py index 6cb9825..f663baa 100644 --- a/lib/lp/bugs/model/bug.py +++ b/lib/lp/bugs/model/bug.py @@ -408,23 +408,12 @@ class Bug(SQLBase, InformationTypeMixin): heat = IntCol(notNull=True, default=0) heat_last_updated = UtcDateTimeCol(default=None) latest_patch_uploaded = UtcDateTimeCol(default=None) - _lock_status = DBEnum( + lock_status = DBEnum( name='lock_status', enum=BugLockStatus, - allow_none=True, default=BugLockStatus.UNLOCKED) + allow_none=False, default=BugLockStatus.UNLOCKED) lock_reason = StringCol(notNull=False, default=None) @property - def lock_status(self): - return ( - BugLockStatus.UNLOCKED if self._lock_status is None - else self._lock_status - ) - - @lock_status.setter - def lock_status(self, value): - self._lock_status = value - - @property def linked_branches(self): return [link.branch for link in self.linked_bugbranches] diff --git a/lib/lp/bugs/tests/test_bug.py b/lib/lp/bugs/tests/test_bug.py index ae35bb0..358b338 100644 --- a/lib/lp/bugs/tests/test_bug.py +++ b/lib/lp/bugs/tests/test_bug.py @@ -7,7 +7,9 @@ from datetime import timedelta import json from lazr.lifecycle.snapshot import Snapshot +from storm.exceptions import NoneError from testtools.matchers import MatchesStructure +from testtools.testcase import ExpectedException from zope.component import getUtility from zope.interface import providedBy from zope.security.management import checkPermission @@ -442,6 +444,12 @@ class TestBugLocking(TestCaseWithFactory): self.assertEqual(BugLockStatus.UNLOCKED, bug.lock_status) self.assertIsNone(bug.lock_reason) + def test_bug_lock_status_cannot_be_none(self): + bug = self.factory.makeBug(owner=self.person, target=self.target) + self.assertEqual(BugLockStatus.UNLOCKED, bug.lock_status) + with ExpectedException(NoneError): + removeSecurityProxy(bug).lock_status = None + def test_bug_locking_when_bug_already_locked(self): bug = self.factory.makeBug(owner=self.person, target=self.target) with person_logged_in(self.target.owner): diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py index 6067080..f1c75ad 100644 --- a/lib/lp/scripts/garbo.py +++ b/lib/lp/scripts/garbo.py @@ -51,7 +51,6 @@ from zope.component import getUtility from zope.security.proxy import removeSecurityProxy from lp.answers.model.answercontact import AnswerContact -from lp.bugs.enums import BugLockStatus from lp.bugs.interfaces.bug import IBugSet from lp.bugs.model.bug import Bug from lp.bugs.model.bugattachment import BugAttachment @@ -1785,42 +1784,6 @@ class RevisionStatusReportPruner(BulkPruner): RevisionStatusArtifactType.BINARY.value) -class PopulateBugLockStatusDefaultUnlocked(TunableLoop): - """ - Populates Bug.lock_status to BugLockStatus.UNLOCKED if not set - """ - - maximum_chunk_size = 5000 - - def __init__(self, log, abort_time=None): - super().__init__(log, abort_time) - self.store = IMasterStore(Bug) - self.start_at = 1 - - def findBugsLockStatusNone(self): - return self.store.find( - Bug.id, - Bug.id >= self.start_at, - Bug._lock_status == None, - ).order_by(Bug.id) - - def setBugLockStatusUnlocked(self, bug_ids): - self.store.find(Bug, Bug.id.is_in(bug_ids)).set( - _lock_status=BugLockStatus.UNLOCKED - ) - - def isDone(self): - return self.findBugsLockStatusNone().is_empty() - - def __call__(self, chunk_size): - bug_ids = [ - bug_id for bug_id in self.findBugsLockStatusNone()[:chunk_size] - ] - self.setBugLockStatusUnlocked(bug_ids) - self.start_at = bug_ids[-1] + 1 - transaction.commit() - - class BaseDatabaseGarbageCollector(LaunchpadCronScript): """Abstract base class to run a collection of TunableLoops.""" script_name = None # Script name for locking and database user. Override. @@ -2112,7 +2075,6 @@ class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector): OCIFilePruner, ObsoleteBugAttachmentPruner, OldTimeLimitedTokenDeleter, - PopulateBugLockStatusDefaultUnlocked, PopulateSnapBuildStoreRevision, POTranslationPruner, PreviewDiffPruner, diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py index 99f36cd..90c4bb3 100644 --- a/lib/lp/scripts/tests/test_garbo.py +++ b/lib/lp/scripts/tests/test_garbo.py @@ -101,7 +101,6 @@ from lp.scripts.garbo import ( load_garbo_job_state, LoginTokenPruner, OpenIDConsumerAssociationPruner, - PopulateBugLockStatusDefaultUnlocked, PopulateSnapBuildStoreRevision, ProductVCSPopulator, save_garbo_job_state, @@ -2110,21 +2109,6 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory): list(getUtility( IRevisionStatusReportSet).findByRepository(repo2))) - def test_PopulateBugLockStatusDefaultUnlocked(self): - switch_dbuser('testadmin') - populator = PopulateBugLockStatusDefaultUnlocked(log=None) - for _ in range(5): - bug = self.factory.makeBug() - removeSecurityProxy(bug).lock_status = None - - result_set = populator.findBugsLockStatusNone() - self.assertGreater(result_set.count(), 0) - - self.runDaily() - - result_set = populator.findBugsLockStatusNone() - self.assertTrue(result_set.is_empty()) - class TestGarboTasks(TestCaseWithFactory): layer = LaunchpadZopelessLayer
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp