feat!: make client.request return response object rather than content

There's lots of things you might want to do with the response object
other than read the comment. As `simple-github` is meant to just be very
basic middleware, we shoudn't make assumptions about what users need
from the response.

Issue: #40

README.md

@@ -43,19 +43,31 @@ In the simplest case, you can provide an access token to use:
 from simple_github import TokenClient
 token = "<access token>"
 async with TokenClient(token) as session:
-    await session.get("/octocat")
+    resp = await session.get("/octocat")
+    resp.raise_for_status()
+    data = await resp.json()
+    await resp.close()
 ```
 
+The return value is an [aiohttp.ClientResponse][0] object.
+
 If calling synchronously, simply remove the `async` / `await` from the
 examples:
 
 ```python
 from simple_github import TokenClient
 token = "<access token>"
 with TokenClient(token) as session:
-    session.get("/octocat")
+    resp = session.get("/octocat")
+    resp.raise_for_status()
+    data = resp.json()
 ```
 
+In this case the return value is a [requests.Response][1] object.
+
+[0]: https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.ClientResponse
+[1]: https://requests.readthedocs.io/en/latest/api/#requests.Response
+
 ### Authenticate as a Github App installation
 
 To authenticate as an app installation you'll need:
@@ -73,14 +85,14 @@ privkey = "<private key>"
 owner = "mozilla-releng"
 
 async with AppClient(app_id, privkey, owner=owner) as session:
-    await session.get("/octocat")
+    resp = await session.get("/octocat")
 ```
 
 You can also specify repositories if you want to restrict access.
 
 ```python
 async with AppClient(app_id, privkey, owner=owner, repositories=["simple-github"]) as session:
-    await session.get("/octocat")
+    resp = await session.get("/octocat")
 ```
 
 ### Authenticate as a Github App
@@ -90,7 +102,7 @@ administering the app. To do this, simply omit the `owner` argument.
 
 ```python
 async with AppClient(app_id, privkey) as session:
-    await session.get("/octocat")
+    resp = await session.get("/octocat")
 ```
 
 ### Query the REST API
@@ -101,7 +113,9 @@ query it by passing in the path fragment to `session.get` or `session.post`.
 For example, you can list pull requests with a `GET` request:
 
 ```python
-pulls = await session.get("/repos/mozilla-releng/simple-github/pulls")
+resp = await session.get("/repos/mozilla-releng/simple-github/pulls")
+pulls = await resp.json()
+await resp.close()
 open_pulls = [p for p in pulls if p["state"] == "open"]
 ```
 

src/simple_github/auth.py

@@ -129,7 +129,9 @@ async def _get_installation_id(self) -> str:
         Returns:
             str: The app's installation id.
         """
-        installations = await self._client.get("/app/installations")
+        async with await self._client.get("/app/installations") as response:
+            response.raise_for_status()
+            installations = await response.json()
         assert isinstance(installations, list)
 
         for installation in installations:
@@ -164,7 +166,9 @@ async def _gen_installation_token(self) -> AsyncGenerator[str, None]:
             data["repositories"] = self.repositories
 
         async def _gentoken() -> str:
-            result = await self._client.post(query, data=data)
+            response = await self._client.post(query, data=data)
+            response.raise_for_status()
+            result = await response.json()
             assert isinstance(result, dict)
             return result["token"]
 

src/simple_github/client.py

@@ -1,24 +1,24 @@
 import asyncio
 import json
 from abc import abstractmethod
-from typing import TYPE_CHECKING, Any, Coroutine, Dict, List, Optional, Union
+from typing import TYPE_CHECKING, Any, Coroutine, Dict, Optional, Union
 
-from aiohttp import ClientSession, ContentTypeError
+from aiohttp import ClientResponse, ClientSession
 from gql import Client as GqlClient
 from gql import gql
 from gql.client import ReconnectingAsyncClientSession, SyncClientSession
 from gql.transport.aiohttp import AIOHTTPTransport
 from gql.transport.requests import RequestsHTTPTransport
+from requests import Response as RequestsResponse
 from requests import Session
-from requests.exceptions import JSONDecodeError
 
 if TYPE_CHECKING:
     from simple_github.auth import Auth
 
 GITHUB_API_ENDPOINT = "https://api.github.com"
 GITHUB_GRAPHQL_ENDPOINT = "https://api.github.com/graphql"
 
-Response = Union[Dict[str, Any], List[Any], str]
+Response = Union[RequestsResponse, ClientResponse]
 RequestData = Optional[Dict[str, Any]]
 
 # Implementations of the base class can be either sync or async.
@@ -120,7 +120,7 @@ def _get_requests_session(self) -> Session:
         assert session.transport.session
         return session.transport.session
 
-    def request(self, method: str, query: str, **kwargs) -> Response:
+    def request(self, method: str, query: str, **kwargs) -> RequestsResponse:
         """Make a request to Github's REST API.
 
         Args:
@@ -136,14 +136,9 @@ def request(self, method: str, query: str, **kwargs) -> Response:
         session = self._get_requests_session()
 
         with session.request(method, url, **kwargs) as resp:
-            if not resp.ok:
-                resp.raise_for_status()
-            try:
-                return resp.json()
-            except JSONDecodeError:
-                return resp.text.strip('"')
-
-    def get(self, query: str) -> Response:
+            return resp
+
+    def get(self, query: str) -> RequestsResponse:
         """Make a GET request to Github's REST API.
 
         Args:
@@ -154,7 +149,7 @@ def get(self, query: str) -> Response:
         """
         return self.request("GET", query)
 
-    def post(self, query: str, data: RequestData = None) -> Response:
+    def post(self, query: str, data: RequestData = None) -> RequestsResponse:
         """Make a POST request to Github's REST API.
 
         Args:
@@ -166,7 +161,7 @@ def post(self, query: str, data: RequestData = None) -> Response:
         """
         return self.request("POST", query, data=json.dumps(data))
 
-    def put(self, query: str, data: RequestData = None) -> Response:
+    def put(self, query: str, data: RequestData = None) -> RequestsResponse:
         """Make a PUT request to Github's REST API.
 
         Args:
@@ -178,7 +173,7 @@ def put(self, query: str, data: RequestData = None) -> Response:
         """
         return self.request("PUT", query, data=json.dumps(data))
 
-    def patch(self, query: str, data: RequestData = None) -> Response:
+    def patch(self, query: str, data: RequestData = None) -> RequestsResponse:
         """Make a PATCH request to Github's REST API.
 
         Args:
@@ -263,7 +258,7 @@ async def _get_aiohttp_session(self) -> ClientSession:
         assert session.transport.session
         return session.transport.session
 
-    async def request(self, method: str, query: str, **kwargs: Any) -> Response:
+    async def request(self, method: str, query: str, **kwargs: Any) -> ClientResponse:
         """Make a request to Github's REST API.
 
         Args:
@@ -277,16 +272,9 @@ async def request(self, method: str, query: str, **kwargs: Any) -> Response:
         """
         url = f"{GITHUB_API_ENDPOINT}/{query.lstrip('/')}"
         session = await self._get_aiohttp_session()
+        return await session.request(method, url, **kwargs)
 
-        async with session.request(method, url, **kwargs) as resp:
-            if not resp.ok:
-                resp.raise_for_status()
-            try:
-                return await resp.json()
-            except ContentTypeError:
-                return (await resp.text()).strip('"')
-
-    async def get(self, query: str) -> Response:
+    async def get(self, query: str) -> ClientResponse:
         """Make a GET request to Github's REST API.
 
         Args:
@@ -297,7 +285,7 @@ async def get(self, query: str) -> Response:
         """
         return await self.request("GET", query)
 
-    async def post(self, query: str, data: RequestData = None) -> Response:
+    async def post(self, query: str, data: RequestData = None) -> ClientResponse:
         """Make a POST request to Github's REST API.
 
         Args:
@@ -309,7 +297,7 @@ async def post(self, query: str, data: RequestData = None) -> Response:
         """
         return await self.request("POST", query, data=json.dumps(data))
 
-    async def put(self, query: str, data: RequestData = None) -> Response:
+    async def put(self, query: str, data: RequestData = None) -> ClientResponse:
         """Make a PUT request to Github's REST API.
 
         Args:
@@ -321,7 +309,7 @@ async def put(self, query: str, data: RequestData = None) -> Response:
         """
         return await self.request("PUT", query, data=json.dumps(data))
 
-    async def patch(self, query: str, data: RequestData = None) -> Response:
+    async def patch(self, query: str, data: RequestData = None) -> ClientResponse:
         """Make a PATCH request to Github's REST API.
 
         Args:

test/test_client.py

@@ -93,60 +93,70 @@ async def test_async_client_rest(aioresponses, async_client):
     url = f"{GITHUB_API_ENDPOINT}/octocat"
 
     aioresponses.get(url, status=200, payload={"answer": 42})
-    result = await client.get("/octocat")
+    resp = await client.get("/octocat")
+    result = await resp.json()
     assert result == {"answer": 42}
 
     aioresponses.post(url, status=200, payload={"answer": 42})
-    result = await client.post("/octocat", data={"foo": "bar"})
+    resp = await client.post("/octocat", data={"foo": "bar"})
+    result = await resp.json()
     assert result == {"answer": 42}
 
     aioresponses.put(url, status=200, payload={"answer": 42})
-    result = await client.put("/octocat", data={"foo": "bar"})
+    resp = await client.put("/octocat", data={"foo": "bar"})
+    result = await resp.json()
     assert result == {"answer": 42}
 
     aioresponses.patch(url, status=200, payload={"answer": 42})
-    result = await client.patch("/octocat", data={"foo": "bar"})
+    resp = await client.patch("/octocat", data={"foo": "bar"})
+    result = await resp.json()
     assert result == {"answer": 42}
 
     aioresponses.delete(url, status=200)
-    result = await client.delete("/octocat")
+    await client.delete("/octocat")
     aioresponses.assert_called_with(url, "DELETE", data="null")
 
     aioresponses.get(url, status=401)
     with pytest.raises(ClientResponseError):
-        await client.get("/octocat")
+        resp = await client.get("/octocat")
+        resp.raise_for_status()
 
 
 def test_sync_client_rest(responses, sync_client):
     client = sync_client
     url = f"{GITHUB_API_ENDPOINT}/octocat"
 
     responses.get(url, status=200, json={"answer": 42})
-    result = client.get("/octocat")
+    resp = client.get("/octocat")
+    result = resp.json()
     assert result == {"answer": 42}
 
     responses.post(url, status=200, json={"answer": 42})
-    result = client.post("/octocat", data={"foo": "bar"})
+    resp = client.post("/octocat", data={"foo": "bar"})
+    result = resp.json()
     assert result == {"answer": 42}
 
     responses.put(url, status=200, json={"answer": 42})
-    result = client.put("/octocat", data={"foo": "bar"})
+    resp = client.put("/octocat", data={"foo": "bar"})
+    result = resp.json()
     assert result == {"answer": 42}
 
     responses.patch(url, status=200, json={"answer": 42})
-    result = client.patch("/octocat", data={"foo": "bar"})
+    resp = client.patch("/octocat", data={"foo": "bar"})
+    result = resp.json()
     assert result == {"answer": 42}
 
     responses.delete(url, status=200)
-    result = client.delete("/octocat")
+    client.delete("/octocat")
     resp = responses.calls[-1].response
     assert resp.url == url
     assert resp.request.method == "DELETE"
     assert resp.status_code == 200
 
     responses.get(url, status=401)
     with pytest.raises(HTTPError):
-        client.get("/octocat")
+        resp = client.get("/octocat")
+        resp.raise_for_status()
 
 
 @pytest.mark.asyncio
@@ -159,7 +169,8 @@ async def test_async_client_rest_with_text(aioresponses, async_client):
         status=200,
         payload=text,
     )
-    result = await client.get("/octocat")
+    resp = await client.get("/octocat")
+    result = (await resp.text()).strip('"')
     assert result == text
 
 
@@ -172,7 +183,8 @@ def test_sync_client_rest_with_text(responses, sync_client):
         status=200,
         json=text,
     )
-    result = client.get("/octocat")
+    resp = client.get("/octocat")
+    result = resp.json()
     assert result == text
 
 

test/test_simple_github.py

@@ -11,7 +11,8 @@ async def test_token_client(aioresponses):
     )
 
     async with TokenClient("abc") as client:
-        result = await client.get("/octocat")
+        resp = await client.get("/octocat")
+        result = await resp.json()
         assert result == {"foo": "bar"}
 
 
@@ -22,7 +23,8 @@ async def test_app_client(aioresponses, privkey):
     )
 
     async with AppClient(id=123, privkey=privkey) as client:
-        result = await client.get("/octocat")
+        resp = await client.get("/octocat")
+        result = await resp.json()
         assert result == {"foo": "bar"}
 
 
@@ -46,5 +48,6 @@ async def test_app_installation_client(aioresponses, privkey):
     )
 
     async with AppClient(id=123, privkey=privkey, owner=owner) as client:
-        result = await client.get("/octocat")
+        resp = await client.get("/octocat")
+        result = await resp.json()
         assert result == {"foo": "bar"}