Repository: ably/ably-python (python)

Issue:
Add mandatory version param for `Rest.request`
See https://github.com/ably/specification/pull/89

Pull request:
refactor!: add mandatory version param to `Rest.request`
Resolves #494 

See [spec change](https://github.com/ably/specification/commit/ebbf1bd9ede206b3cf49b4aafc22524d4cf8a933) and [ably-js implementation](https://github.com/ably/ably-js/pull/1231)

Patch:
diff --git a/ably/http/http.py b/ably/http/http.py
index 054fe00..440bf0c 100644
--- a/ably/http/http.py
+++ b/ably/http/http.py
@@ -44,18 +44,19 @@ def reauth_if_expired(func):
 
 
 class Request:
-    def __init__(self, method='GET', url='/', headers=None, body=None,
+    def __init__(self, method='GET', url='/', version=None, headers=None, body=None,
                  skip_auth=False, raise_on_error=True):
         self.__method = method
         self.__headers = headers or {}
         self.__body = body
         self.__skip_auth = skip_auth
         self.__url = url
+        self.__version = version
         self.raise_on_error = raise_on_error
 
     def with_relative_url(self, relative_url):
         url = urljoin(self.url, relative_url)
-        return Request(self.method, url, self.headers, self.body,
+        return Request(self.method, url, self.version, self.headers, self.body,
                        self.skip_auth, self.raise_on_error)
 
     @property
@@ -78,6 +79,10 @@ class Request:
     def skip_auth(self):
         return self.__skip_auth
 
+    @property
+    def version(self):
+        return self.__version
+
 
 class Response:
     """
@@ -152,16 +157,16 @@ class Http:
         return hosts
 
     @reauth_if_expired
-    async def make_request(self, method, path, headers=None, body=None,
+    async def make_request(self, method, path, version=None, headers=None, body=None,
                            skip_auth=False, timeout=None, raise_on_error=True):
 
         if body is not None and type(body) not in (bytes, str):
             body = self.dump_body(body)
 
         if body:
-            all_headers = HttpUtils.default_post_headers(self.options.use_binary_protocol)
+            all_headers = HttpUtils.default_post_headers(self.options.use_binary_protocol, version=version)
         else:
-            all_headers = HttpUtils.default_get_headers(self.options.use_binary_protocol)
+            all_headers = HttpUtils.default_get_headers(self.options.use_binary_protocol, version=version)
 
         if not skip_auth:
             if self.auth.auth_mechanism == Auth.Method.BASIC and self.preferred_scheme.lower() == 'http':
diff --git a/ably/http/httputils.py b/ably/http/httputils.py
index 53a583a..20c7131 100644
--- a/ably/http/httputils.py
+++ b/ably/http/httputils.py
@@ -14,8 +14,8 @@ class HttpUtils:
     }
 
     @staticmethod
-    def default_get_headers(binary=False):
-        headers = HttpUtils.default_headers()
+    def default_get_headers(binary=False, version=None):
+        headers = HttpUtils.default_headers(version=version)
         if binary:
             headers["Accept"] = HttpUtils.mime_types['binary']
         else:
@@ -23,8 +23,8 @@ class HttpUtils:
         return headers
 
     @staticmethod
-    def default_post_headers(binary=False):
-        headers = HttpUtils.default_get_headers(binary=binary)
+    def default_post_headers(binary=False, version=None):
+        headers = HttpUtils.default_get_headers(binary=binary, version=version)
         headers["Content-Type"] = headers["Accept"]
         return headers
 
@@ -35,8 +35,10 @@ class HttpUtils:
         }
 
     @staticmethod
-    def default_headers():
+    def default_headers(version=None):
+        if version is None:
+            version = ably.api_version
         return {
-            "X-Ably-Version": ably.api_version,
+            "X-Ably-Version": version,
             "Ably-Agent": 'ably-python/%s python/%s' % (ably.lib_version, platform.python_version())
         }
diff --git a/ably/http/paginatedresult.py b/ably/http/paginatedresult.py
index fffcabf..6421251 100644
--- a/ably/http/paginatedresult.py
+++ b/ably/http/paginatedresult.py
@@ -77,11 +77,11 @@ class PaginatedResult:
         return await self.paginated_query_with_request(self.__http, rel_req, self.__response_processor)
 
     @classmethod
-    async def paginated_query(cls, http, method='GET', url='/', body=None,
+    async def paginated_query(cls, http, method='GET', url='/', version=None, body=None,
                               headers=None, response_processor=None,
                               raise_on_error=True):
         headers = headers or {}
-        req = Request(method, url, body=body, headers=headers, skip_auth=False,
+        req = Request(method, url, version=version, body=body, headers=headers, skip_auth=False,
                       raise_on_error=raise_on_error)
         return await cls.paginated_query_with_request(http, req, response_processor)
 
@@ -89,9 +89,9 @@ class PaginatedResult:
     async def paginated_query_with_request(cls, http, request, response_processor,
                                            raise_on_error=True):
         response = await http.make_request(
-            request.method, request.url, headers=request.headers,
-            body=request.body, skip_auth=request.skip_auth,
-            raise_on_error=request.raise_on_error)
+            request.method, request.url, version=request.version,
+            headers=request.headers, body=request.body,
+            skip_auth=request.skip_auth, raise_on_error=request.raise_on_error)
 
         items = response_processor(response)
 
diff --git a/ably/rest/rest.py b/ably/rest/rest.py
index 64b2c68..a42ba2f 100644
--- a/ably/rest/rest.py
+++ b/ably/rest/rest.py
@@ -119,7 +119,11 @@ class AblyRest:
     def push(self):
         return self.__push
 
-    async def request(self, method: str, path: str, params: Optional[dict] = None, body=None, headers=None):
+    async def request(self, method: str, path: str, version: str, params:
+                      Optional[dict] = None, body=None, headers=None):
+        if version is None:
+            raise AblyException("No version parameter", 400, 40000)
+
         url = path
         if params:
             url += '?' + urlencode(params)
@@ -133,7 +137,7 @@ class AblyRest:
             return items
 
         return await HttpPaginatedResponse.paginated_query(
-            self.http, method, url, body=body, headers=headers,
+            self.http, method, url, version=version, body=body, headers=headers,
             response_processor=response_processor,
             raise_on_error=False)
 


Test patch:
diff --git a/test/ably/rest/resthttp_test.py b/test/ably/rest/resthttp_test.py
index db219b5..4929bdf 100644
--- a/test/ably/rest/resthttp_test.py
+++ b/test/ably/rest/resthttp_test.py
@@ -25,7 +25,7 @@ class TestRestHttp(BaseAsyncTestCase):
 
         with mock.patch('httpx.AsyncClient.send', side_effect=httpx.RequestError('')) as send_mock:
             with pytest.raises(httpx.RequestError):
-                await ably.http.make_request('GET', '/', skip_auth=True)
+                await ably.http.make_request('GET', '/', version=Defaults.protocol_version, skip_auth=True)
 
             assert send_mock.call_count == Defaults.http_max_retry_count
             assert send_mock.call_args == mock.call(mock.ANY)
diff --git a/test/ably/rest/restrequest_test.py b/test/ably/rest/restrequest_test.py
index 2824d57..d0c9ad9 100644
--- a/test/ably/rest/restrequest_test.py
+++ b/test/ably/rest/restrequest_test.py
@@ -4,6 +4,7 @@ import respx
 
 from ably import AblyRest
 from ably.http.paginatedresult import HttpPaginatedResponse
+from ably.transport.defaults import Defaults
 from test.ably.testapp import TestApp
 from test.ably.utils import BaseAsyncTestCase
 from test.ably.utils import VaryByProtocolTestsMetaclass, dont_vary_protocol
@@ -21,7 +22,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
         self.path = '/channels/%s/messages' % self.channel
         for i in range(20):
             body = {'name': 'event%s' % i, 'data': 'lorem ipsum %s' % i}
-            await self.ably.request('POST', self.path, body=body)
+            await self.ably.request('POST', self.path, body=body, version=Defaults.protocol_version)
 
     async def asyncTearDown(self):
         await self.ably.close()
@@ -32,7 +33,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
 
     async def test_post(self):
         body = {'name': 'test-post', 'data': 'lorem ipsum'}
-        result = await self.ably.request('POST', self.path, body=body)
+        result = await self.ably.request('POST', self.path, body=body, version=Defaults.protocol_version)
 
         assert isinstance(result, HttpPaginatedResponse)  # RSC19d
         # HP3
@@ -43,7 +44,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
 
     async def test_get(self):
         params = {'limit': 10, 'direction': 'forwards'}
-        result = await self.ably.request('GET', self.path, params=params)
+        result = await self.ably.request('GET', self.path, params=params, version=Defaults.protocol_version)
 
         assert isinstance(result, HttpPaginatedResponse)  # RSC19d
 
@@ -68,7 +69,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
 
     @dont_vary_protocol
     async def test_not_found(self):
-        result = await self.ably.request('GET', '/not-found')
+        result = await self.ably.request('GET', '/not-found', version=Defaults.protocol_version)
         assert isinstance(result, HttpPaginatedResponse)  # RSC19d
         assert result.status_code == 404             # HP4
         assert result.success is False               # HP5
@@ -76,7 +77,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
     @dont_vary_protocol
     async def test_error(self):
         params = {'limit': 'abc'}
-        result = await self.ably.request('GET', self.path, params=params)
+        result = await self.ably.request('GET', self.path, params=params, version=Defaults.protocol_version)
         assert isinstance(result, HttpPaginatedResponse)  # RSC19d
         assert result.status_code == 400  # HP4
         assert not result.success
@@ -86,7 +87,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
     async def test_headers(self):
         key = 'X-Test'
         value = 'lorem ipsum'
-        result = await self.ably.request('GET', '/time', headers={key: value})
+        result = await self.ably.request('GET', '/time', headers={key: value}, version=Defaults.protocol_version)
         assert result.response.request.headers[key] == value
 
     # RSC19e
@@ -97,7 +98,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
         ably = AblyRest(token="foo", http_request_timeout=timeout)
         assert ably.http.http_request_timeout == timeout
         with pytest.raises(httpx.ReadTimeout):
-            await ably.request('GET', '/time')
+            await ably.request('GET', '/time', version=Defaults.protocol_version)
         await ably.close()
 
         default_endpoint = 'https://sandbox-rest.ably.io/time'
@@ -112,7 +113,7 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
             }
             default_route.side_effect = httpx.ConnectError('')
             fallback_route.return_value = httpx.Response(200, headers=headers, text='[123]')
-            await ably.request('GET', '/time')
+            await ably.request('GET', '/time', version=Defaults.protocol_version)
         await ably.close()
 
         # Bad host, no Fallback
@@ -122,5 +123,10 @@ class TestRestRequest(BaseAsyncTestCase, metaclass=VaryByProtocolTestsMetaclass)
                         tls_port=self.test_vars["tls_port"],
                         tls=self.test_vars["tls"])
         with pytest.raises(httpx.ConnectError):
-            await ably.request('GET', '/time')
+            await ably.request('GET', '/time', version=Defaults.protocol_version)
         await ably.close()
+
+    async def test_version(self):
+        version = "150"  # chosen arbitrarily
+        result = await self.ably.request('GET', '/time', "150")
+        assert result.response.request.headers["X-Ably-Version"] == version