jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/541089 ) Change subject: [IMPR] Rewrite log_page/unlock_page implementation ...................................................................... [IMPR] Rewrite log_page/unlock_page implementation - Use threading.Condition context manager instead of try/finally - move test_lock_page from TestSiteGenerators to TestLockingPage class because this test is not related to a generator Change-Id: I1ff2c39d43b78d7e71d27720e8ec83e3a48a6925 --- M pywikibot/site.py M tests/site_tests.py 2 files changed, 24 insertions(+), 43 deletions(-) Approvals: Zhuyifei1999: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/site.py b/pywikibot/site.py index fac1d66..f8950f6 100644 --- a/pywikibot/site.py +++ b/pywikibot/site.py @@ -6,7 +6,7 @@ groups of wikis on the same topic in different languages. """ # -# (C) Pywikibot team, 2008-2019 +# (C) Pywikibot team, 2008-2020 # # Distributed under the terms of the MIT license. # @@ -780,7 +780,7 @@ self.code in self.family.use_hard_category_redirects) # following are for use with lock_page and unlock_page methods - self._pagemutex = threading.Lock() + self._pagemutex = threading.Condition() self._locked_pages = set() @deprecated(since='20141225') @@ -880,7 +880,7 @@ def __setstate__(self, attrs): """Restore things removed in __getstate__.""" self.__dict__.update(attrs) - self._pagemutex = threading.Lock() + self._pagemutex = threading.Condition() def user(self): """Return the currently-logged in bot username, or None.""" @@ -1065,29 +1065,12 @@ """ title = page.title(with_section=False) - - self._pagemutex.acquire() - try: + with self._pagemutex: while title in self._locked_pages: if not block: raise PageInUse(title) - - # The mutex must be released so that page can be unlocked - self._pagemutex.release() - time.sleep(.25) - self._pagemutex.acquire() - + self._pagemutex.wait() self._locked_pages.add(title) - finally: - # time.sleep may raise an exception from signal handler (eg: - # KeyboardInterrupt) while the lock is released, and there is no - # reason to acquire the lock again given that our caller will - # receive the exception. The state of the lock is therefore - # undefined at the point of this finally block. - try: - self._pagemutex.release() - except RuntimeError: - pass def unlock_page(self, page): """ @@ -1097,11 +1080,9 @@ @type page: pywikibot.Page """ - self._pagemutex.acquire() - try: + with self._pagemutex: self._locked_pages.discard(page.title(with_section=False)) - finally: - self._pagemutex.release() + self._pagemutex.notify_all() def disambcategory(self): """Return Category in which disambig pages are listed.""" diff --git a/tests/site_tests.py b/tests/site_tests.py index d700c51..afa9a02 100644 --- a/tests/site_tests.py +++ b/tests/site_tests.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """Tests for the site module.""" # -# (C) Pywikibot team, 2008-2019 +# (C) Pywikibot team, 2008-2020 # # Distributed under the terms of the MIT license. # @@ -974,19 +974,6 @@ self.assertIsInstance(link, pywikibot.Page) self.assertIn(link.namespace(), (2, 3)) - def test_lock_page(self): - """Test the site.lock_page() and site.unlock_page() method.""" - site = self.get_site() - p1 = pywikibot.Page(site, 'Foo') - - site.lock_page(page=p1, block=True) - self.assertRaises(pywikibot.site.PageInUse, site.lock_page, page=p1, - block=False) - site.unlock_page(page=p1) - # verify it's unlocked - site.lock_page(page=p1, block=False) - site.unlock_page(page=p1) - def test_protectedpages_create(self): """Test that protectedpages returns protected page titles.""" if self.site.mw_version < '1.15': @@ -1089,7 +1076,7 @@ self.assertRaises(AssertionError, func, 'm', 2, 1, True, True) -class TestThreadsLockingPage(DefaultSiteTestCase): +class TestLockingPage(DefaultSiteTestCase): """Test cases for lock/unlock a page within threads.""" cached = True @@ -1102,8 +1089,8 @@ time.sleep(wait) page.site.unlock_page(page=page) - def test_lock_page(self): - """Test the site.lock_page() and site.unlock_page() method.""" + def test_threads_locking_page(self): + """Test lock_page and unlock_page methods for multiple threads.""" # Start few threads threads = [] for i in range(5): @@ -1121,6 +1108,19 @@ self.assertFalse(thread.is_alive(), 'test page is still locked') + def test_lock_page(self): + """Test the site.lock_page() and site.unlock_page() method.""" + site = self.get_site() + p1 = pywikibot.Page(site, 'Foo') + + site.lock_page(page=p1, block=True) + self.assertRaises(pywikibot.site.PageInUse, site.lock_page, page=p1, + block=False) + site.unlock_page(page=p1) + # verify it's unlocked + site.lock_page(page=p1, block=False) + site.unlock_page(page=p1) + class TestSiteGeneratorsUsers(DefaultSiteTestCase): """Test cases for Site methods with users.""" -- To view, visit https://gerrit.wikimedia.org/r/541089 To unsubscribe, or for help writing mail filters, visit https://gerrit.wikimedia.org/r/settings Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1ff2c39d43b78d7e71d27720e8ec83e3a48a6925 Gerrit-Change-Number: 541089 Gerrit-PatchSet: 4 Gerrit-Owner: Xqt &lt;info(a)gno.de&gt; Gerrit-Reviewer: Count Count &lt;countvoncount123456(a)gmail.com&gt; Gerrit-Reviewer: Merlijn van Deen &lt;valhallasw(a)arctus.nl&gt; Gerrit-Reviewer: Mineo &lt;themineo(a)gmail.com&gt; Gerrit-Reviewer: Xqt &lt;info(a)gno.de&gt; Gerrit-Reviewer: Zhuyifei1999 &lt;zhuyifei1999(a)gmail.com&gt; Gerrit-Reviewer: jenkins-bot (75)