Skip to content

Commit

Permalink
make init params of HttpRequest/HttpResponse kw-only except for url
Browse files Browse the repository at this point in the history
  • Loading branch information
BurnzZ committed Apr 11, 2022
1 parent 29f84c6 commit d88a0e5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 13 deletions.
10 changes: 5 additions & 5 deletions tests/test_page_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def test_http_response_json():
response = HttpResponse(url, body=b'{"key": "value"}')
assert response.json() == {"key": "value"}

response = HttpResponse(url, '{"ключ": "значение"}'.encode("utf8"))
response = HttpResponse(url, body='{"ключ": "значение"}'.encode("utf8"))
assert response.json() == {"ключ": "значение"}


Expand All @@ -274,7 +274,7 @@ def test_http_response_text():
"""
text = "œ is a Weird Character"
body = HttpResponseBody(b"\x9c is a Weird Character")
response = HttpResponse("http://example.com", body)
response = HttpResponse("http://example.com", body=body)

assert response.text == text

Expand All @@ -293,7 +293,7 @@ def test_http_headers_declared_encoding(headers, encoding):
headers = HttpResponseHeaders(headers)
assert headers.declared_encoding() == encoding

response = HttpResponse("http://example.com", b'', headers=headers)
response = HttpResponse("http://example.com", body=b'', headers=headers)
assert response.encoding == encoding or HttpResponse._DEFAULT_ENCODING


Expand All @@ -307,14 +307,14 @@ def test_http_response_utf16():


def test_explicit_encoding():
response = HttpResponse("http://www.example.com", "£".encode('utf-8'),
response = HttpResponse("http://www.example.com", body="£".encode('utf-8'),
encoding='utf-8')
assert response.encoding == "utf-8"
assert response.text == "£"


def test_explicit_encoding_invalid():
response = HttpResponse("http://www.example.com", "£".encode('utf-8'),
response = HttpResponse("http://www.example.com", body="£".encode('utf-8'),
encoding='latin1')
assert response.encoding == "latin1"
assert response.text == "£".encode('utf-8').decode("latin1")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def async_mock():
"""Workaround since python 3.7 doesn't ship with asyncmock."""

async def async_test(req):
return HttpResponse(req.url, b"")
return HttpResponse(req.url, body=b"")

mock.MagicMock.__await__ = lambda x: async_test().__await__()

Expand Down
15 changes: 8 additions & 7 deletions web_poet/page_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ class HttpRequest:
"""

url: str = attrs.field()
method: str = attrs.field(default="GET")
method: str = attrs.field(default="GET", kw_only=True)
headers: HttpRequestHeaders = attrs.field(
factory=HttpRequestHeaders, converter=HttpRequestHeaders
factory=HttpRequestHeaders, converter=HttpRequestHeaders, kw_only=True
)
body: HttpRequestBody = attrs.field(
factory=HttpRequestBody, converter=HttpRequestBody
factory=HttpRequestBody, converter=HttpRequestBody, kw_only=True
)


Expand Down Expand Up @@ -190,11 +190,12 @@ class HttpResponse:
"""

url: str = attrs.field()
body: HttpResponseBody = attrs.field(converter=HttpResponseBody)
status: Optional[int] = attrs.field(default=None)
body: HttpResponseBody = attrs.field(converter=HttpResponseBody, kw_only=True)

This comment has been minimized.

Copy link
@kmike

kmike Apr 11, 2022

Member

What do you think about keeping it positional @BurnzZ? It seems most responses have body, and this argument is required, unlike status, headers of encoding.

This comment has been minimized.

Copy link
@BurnzZ

BurnzZ Apr 11, 2022

Author Contributor

I was initially considering HEAD requests but come to think of it, it's the body is almost always present in most cases, as you've pointed out.

Reverted this in cf6e663

status: Optional[int] = attrs.field(default=None, kw_only=True)
headers: HttpResponseHeaders = attrs.field(factory=HttpResponseHeaders,
converter=HttpResponseHeaders)
_encoding: Optional[str] = attrs.field(default=None)
converter=HttpResponseHeaders,
kw_only=True)
_encoding: Optional[str] = attrs.field(default=None, kw_only=True)

_DEFAULT_ENCODING = 'ascii'
_cached_text: Optional[str] = None
Expand Down

0 comments on commit d88a0e5

Please sign in to comment.