search

getsentry / responses

A utility for mocking out the Python Requests library.
Apache License 2.0
4.08k stars 347 forks source link

dev(narugo): add support for headers and binary body in recorder #683

Open narugo1992 opened 8 months ago

narugo1992 commented 8 months ago

Related issue: #682

In addition, after our testing, this version can be compatible with the yaml format of the previous version.

narugo1992 commented 8 months ago

one more thing

Aftering saving the binary responses to yaml file (some images from danbooru, 0.5MB~50MB per image) using this pr, I found it be really slow when calling _all_from_file, upto 5secs to 5minutes each file.

Maybe a better format should be used to optimize it?

I'm cosidering to save the binary body to extra binary files instead of simply put it in yaml with base64 code.

beliaev-maksim commented 8 months ago

Performance will be always an issue if we try to save binaries in yaml

We need to enhance the method to allow to supply file path. Then we can store binaries close to yamls. That will be more human readable and performant

Can you please update the PR with the change

narugo1992 commented 8 months ago

Performance will be always an issue if we try to save binaries in yaml

We need to enhance the method to allow to supply file path. Then we can store binaries close to yamls. That will be more human readable and performant

Can you please update the PR with the change

yep, I am also re-designing these code according to this idea.

draft it for now

narugo1992 commented 8 months ago

@beliaev-maksim now the binary data are saved to files.

For example, if you specify the config file responses.yaml, the binary files will be placed at responses/1.bin, responses/2.bin, etc. And the relative path of binary files will be put into the responses.yaml (the relative paths are relative to responses.yaml's parent directory, so the yaml and binary files can be easily moved to somewhere without any change).

I'm now using the code in this pr for the unittest in my own project, and it works well on ubuntu20.04, Python3.8. Here are some yaml and binary files I created to my unittest (the zip files in this repo): https://huggingface.co/datasets/deepghs/waifuc_responses/tree/main .

I think cross-platform testing is needed now, especially on the Windows platform, and the / in the relative path may cause errors.

narugo1992 commented 8 months ago

Discovered a new issue: 'responses' cannot handle HEAD requests with the Content-Length header properly, causing an IncompleteRead exception to be thrown during length checking in urllib3.

Update: fixed in commit 523857e

narugo1992 commented 8 months ago

@beliaev-maksim @markstory

Now ready for a review, as this version has already been deployed in our own project and is running as expected.

Additionally, all unit tests have passed in Github Actions.