Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: support JSONRepsonse dumps callable return type bytes #3000

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

imnotjames
Copy link

@imnotjames imnotjames commented Sep 27, 2024

the dumps parameter is used to serialize the JSONResponse to text. this is often done with python's built in json.dumps but other times via ujson.dumps and orjson.dumps

however, to use orjson.dumps as suggested in the docs, you have to ignore the suggested type (str) because orjson.dumps returns a bytes

to correct this, the typing for the dumps callable has been switched to be an Callable[..., AnyStr] so either bytes or str returning callable can be passed

this was previously done in #2193 but for some reason not brought over to JSONResponse?

@imnotjames imnotjames marked this pull request as ready for review September 27, 2024 06:05
@imnotjames imnotjames requested a review from a team as a code owner September 27, 2024 06:05
@imnotjames imnotjames changed the title chore: match JSONRepsonse dumps param type to reality chore: match JSONRepsonse dumps callable return type to AnyStr Sep 27, 2024
@imnotjames imnotjames changed the title chore: match JSONRepsonse dumps callable return type to AnyStr chore: support JSONRepsonse dumps callable return type bytes Sep 27, 2024
the `dumps` parameter is used to serialize the `JSONResponse` to text.  this is often done with python's built in `json.dumps` but other times via `orjson.dumps`

however, to use `orjson.dumps`, you have to ignore the suggested type (`str`) because `orjson.dumps` returns a `bytes`

to correct this, the typing for the dumps callable has been switched to be an `AnyStr` so either `bytes` or `str` returning callable can be passed
@@ -59,7 +59,7 @@ def __repr__(self):
class_name = self.__class__.__name__
return f"<{class_name}: {self.status} {self.content_type}>"

def _encode_body(self, data: Optional[AnyStr]):
def _encode_body(self, data: Optional[str | bytes]):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an odd change - without this, everything complained that it should be Sequence[object]?

sanic/response/types.py:334: error: Value of type variable "AnyStr" of "_encode_body" of "BaseHTTPResponse" cannot be "Sequence[object]"  [type-var]

Unfortunately, not familiar enough with mypy for this.

@imnotjames
Copy link
Author

For some reason the type checking is acting up - but it's on things that I don't think are related. Happy to fix as needed with a bit of direction, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant