fix: Fix serialization payload in Request. Fix Docs for Post Request #683
Conversation
vdusek
added
t-tooling
vdusek
left a comment
vdusek
left a comment
There was a problem hiding this comment.
I got just stylistic things... We already discussed it directly, but we should make sure this is going to work on the platform. Let me know once it is tested. Thank you.
|
Thanks @Mantisus! Could you please add a test that shows that the bug has been fixed? |
vdusek
left a comment
vdusek
left a comment
There was a problem hiding this comment.
One last thing, otherwise LGTM, thanks.
Co-authored-by: Vlada Dusek <[email protected]>
| payload: Annotated[ | ||
| HttpPayload | None, | ||
| BeforeValidator(lambda v: v.encode() if isinstance(v, str) else v), | ||
| PlainSerializer(lambda v: v.decode() if isinstance(v, bytes) else None), |
There was a problem hiding this comment.
This is scary - could you return v in the else branch instead?
| got_request = await request_queue_client.get_request(request.id) | ||
|
|
||
| assert request == got_request | ||
| assert request.payload == got_request.payload |
There was a problem hiding this comment.
This comparison is part of Request.__eq__, so this assertion accomplishes nothing.
The problem in #668 is due to the fact that after serialization-deserialization payload does not match, b “test” becomes b “b'test'”. We can add a test to check this case specifically. But in essence, it is sufficient. assert request == got_requestAll the other problems described in #668 are related to incorrectly created payload for the Request. |
### Description Correction of comments related to PR #683 Add new tests for `http_crawler` different forms of `payload` ### Testing The test checks that the `payload` on the client side matches the data received by the server. Also added verification that different types of `payload` are correctly handled on the client side and correctly recognized by the server with appropriate headers ### Checklist - [x] CI passed
3 participants
Description
An incorrect serialization for
payloadis happening now.payloadis represented as a byte string, but bytes cannot be serialized to json. Therefore, when loading and unloading from the queue (_persist_single_request_to_storage,_json_to_request) we get strings of format “b'test'”Also corrected the documentation, as when filling forms the data should be passed not in json format and have the appropriate header.
Issues
Testing
Updated
test_request_state_serializationto also takepayloadinto account when serializing and deserializingRequestChecklist