Repository: mwclient/mwclient

Issue:
Patrol function
Hello, currently some function are commented out in the code :
```
    # def block(self): TODO?
    # def unblock: TODO?
    # def patrol: TODO?
    # def import: TODO?
```
I am very interested in the patrol function, do you plan on adding it ?

By the way, thanks for the library, it's really useful.

Discussion:
AdamWill: This looks solid to me from the test and the API doc. Thanks. Can you rebase it? Looks like it's 3 commits behind.

Patch:
diff --git a/mwclient/client.py b/mwclient/client.py
index f0c8f1b..1ea1dde 100644
--- a/mwclient/client.py
+++ b/mwclient/client.py
@@ -469,22 +469,23 @@ class Site(object):
                                                             ext=self.ext)
 
         while True:
-            try:
-                args = {'files': files, 'headers': headers}
-                for k, v in self.requests.items():
-                    args[k] = v
-                if http_method == 'GET':
-                    args['params'] = data
-                else:
-                    args['data'] = data
+            toraise = None
+            wait_time = 0
+            args = {'files': files, 'headers': headers}
+            for k, v in self.requests.items():
+                args[k] = v
+            if http_method == 'GET':
+                args['params'] = data
+            else:
+                args['data'] = data
 
+            try:
                 stream = self.connection.request(http_method, url, **args)
-
                 if stream.headers.get('x-database-lag'):
                     wait_time = int(stream.headers.get('retry-after'))
                     log.warning('Database lag exceeds max lag. '
                                 'Waiting for {} seconds'.format(wait_time))
-                    sleeper.sleep(wait_time)
+                    # fall through to the sleep
                 elif stream.status_code == 200:
                     return stream.text
                 elif stream.status_code < 500 or stream.status_code > 599:
@@ -496,16 +497,32 @@ class Site(object):
                                 'Retrying in a moment.'
                                 .format(status=stream.status_code,
                                         text=stream.text))
-                    sleeper.sleep()
+                    toraise = "stream"
+                    # fall through to the sleep
 
-            except (requests.exceptions.ConnectionError, requests.exceptions.Timeout):
+            except (
+                requests.exceptions.ConnectionError,
+                requests.exceptions.Timeout
+            ) as err:
                 # In the event of a network problem
                 # (e.g. DNS failure, refused connection, etc),
                 # Requests will raise a ConnectionError exception.
                 if not retry_on_error:
                     raise
                 log.warning('Connection error. Retrying in a moment.')
-                sleeper.sleep()
+                toraise = err
+                # proceed to the sleep
+
+            # all retry paths come here
+            try:
+                sleeper.sleep(wait_time)
+            except errors.MaximumRetriesExceeded:
+                if toraise == "stream":
+                    stream.raise_for_status()
+                elif toraise:
+                    raise toraise
+                else:
+                    raise
 
     def raw_api(self, action, http_method='POST', retry_on_error=True, *args, **kwargs):
         """Send a call to the API.
@@ -1062,9 +1079,47 @@ class Site(object):
 
     # def block(self): TODO?
     # def unblock: TODO?
-    # def patrol: TODO?
     # def import: TODO?
 
+    def patrol(self, rcid=None, revid=None, tags=None):
+        """Patrol a page or a revision. Either ``rcid`` or ``revid`` (but not both) must
+        be given.
+        The ``rcid`` and ``revid`` arguments may be obtained using the
+        :meth:`Site.recentchanges` function.
+
+        API doc: https://www.mediawiki.org/wiki/API:Patrol
+
+        Args:
+            rcid (int): The recentchanges ID to patrol.
+            revid (int): The revision ID to patrol.
+            tags (str): Change tags to apply to the entry in the patrol log. Multiple
+                tags can be given, by separating them with the pipe (|) character.
+
+        Returns:
+            Dict[str, Any]: The API response as a dictionary containing:
+
+            - **rcid** (int): The recentchanges id.
+            - **nsid** (int): The namespace id.
+            - **title** (str): The page title.
+
+        Raises:
+            errors.APIError: The MediaWiki API returned an error.
+
+        Notes:
+            - ``autopatrol`` rights are required in order to use this function.
+            - ``revid`` requires at least MediaWiki 1.22.
+            - ``tags`` requires at least MediaWiki 1.27.
+        """
+        if self.require(1, 17, raise_error=False):
+            token = self.get_token('patrol')
+        else:
+            # For MediaWiki versions earlier than 1.17, the patrol token is the same the
+            # edit token.
+            token = self.get_token('edit')
+
+        result = self.post('patrol', rcid=rcid, revid=revid, tags=tags, token=token)
+        return result['patrol']
+
     # Lists
     def allpages(self, start=None, prefix=None, namespace='0', filterredir='all',
                  minsize=None, maxsize=None, prtype=None, prlevel=None,


Test patch:
diff --git a/test/test_client.py b/test/test_client.py
index c89c3c9..e4b8cda 100644
--- a/test/test_client.py
+++ b/test/test_client.py
@@ -403,6 +403,67 @@ class TestClient(TestCase):
 
         assert repr(site) == '<Site object \'test.wikipedia.org/w/\'>'
 
+    @mock.patch("time.sleep")
+    @responses.activate
+    def test_api_http_error(self, timesleep):
+        # Test error paths in raw_call, via raw_api as it's more
+        # convenient to call. This would be way nicer with pytest
+        # parametrization but you can't use parametrization inside
+        # unittest.TestCase :(
+        site = self.stdSetup()
+        # All HTTP errors should raise HTTPError with or without retries
+        self.httpShouldReturn(body="foo", status=401)
+        with pytest.raises(requests.exceptions.HTTPError):
+            site.raw_api("query", "GET")
+        # for a 4xx response we should *not* retry
+        assert timesleep.call_count == 0
+        with pytest.raises(requests.exceptions.HTTPError):
+            site.raw_api("query", "GET", retry_on_error=False)
+        self.httpShouldReturn(body="foo", status=503)
+        with pytest.raises(requests.exceptions.HTTPError):
+            site.raw_api("query", "GET")
+        # for a 5xx response we *should* retry
+        assert timesleep.call_count == 25
+        timesleep.reset_mock()
+        with pytest.raises(requests.exceptions.HTTPError):
+            site.raw_api("query", "GET", retry_on_error=False)
+        # check we did not retry
+        assert timesleep.call_count == 0
+        # stop sending bad statuses
+        self.httpShouldReturn(body="foo", status=200)
+        # ConnectionError should retry then pass through, takes
+        # advantage of responses raising ConnectionError if you
+        # hit a URL that hasn't been configured. Timeout follows
+        # the same path so we don't bother testing it separately
+        realhost = site.host
+        site.host = "notthere"
+        with pytest.raises(requests.exceptions.ConnectionError):
+            site.raw_api("query", "GET")
+        assert timesleep.call_count == 25
+        timesleep.reset_mock()
+        with pytest.raises(requests.exceptions.ConnectionError):
+            site.raw_api("query", "GET", retry_on_error=False)
+        # check we did not retry
+        assert timesleep.call_count == 0
+
+    @mock.patch("time.sleep")
+    @responses.activate
+    def test_api_dblag(self, timesleep):
+        site = self.stdSetup()
+        # db lag should retry then raise MaximumRetriesExceeded,
+        # even with retry_on_error set
+        self.httpShouldReturn(
+            body="foo",
+            status=200,
+            headers={"x-database-lag": "true", "retry-after": "5"}
+        )
+        with pytest.raises(mwclient.errors.MaximumRetriesExceeded):
+            site.raw_api("query", "GET")
+        assert timesleep.call_count == 25
+        timesleep.reset_mock()
+        with pytest.raises(mwclient.errors.MaximumRetriesExceeded):
+            site.raw_api("query", "GET", retry_on_error=False)
+        assert timesleep.call_count == 25
 
 class TestLogin(TestCase):
 
@@ -759,5 +820,43 @@ class TestClientGetTokens(TestCase):
 
         assert self.site.tokens['edit'] == 'sometoken'
 
+
+class TestClientPatrol(TestCase):
+
+    def setUp(self):
+        self.raw_call = mock.patch('mwclient.client.Site.raw_call').start()
+
+    def configure(self, version='1.24'):
+        self.raw_call.return_value = self.metaResponseAsJson(version=version)
+        self.site = mwclient.Site('test.wikipedia.org')
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    @mock.patch('mwclient.client.Site.get_token')
+    def test_patrol(self, get_token):
+        self.configure('1.24')
+        get_token.return_value = 'sometoken'
+        patrol_response = {"patrol": {"rcid": 12345, "ns": 0, "title": "Foo"}}
+        self.raw_call.return_value = json.dumps(patrol_response)
+
+        resp = self.site.patrol(12345)
+
+        assert resp == patrol_response["patrol"]
+        get_token.assert_called_once_with('patrol')
+
+    @mock.patch('mwclient.client.Site.get_token')
+    def test_patrol_on_mediawiki_below_1_17(self, get_token):
+        self.configure('1.16')
+        get_token.return_value = 'sometoken'
+        patrol_response = {"patrol": {"rcid": 12345, "ns": 0, "title": "Foo"}}
+        self.raw_call.return_value = json.dumps(patrol_response)
+
+        resp = self.site.patrol(12345)
+
+        assert resp == patrol_response["patrol"]
+        get_token.assert_called_once_with('edit')
+
+
 if __name__ == '__main__':
     unittest.main()