getsentry
dev(narugo): add support for headers and binary body in recorder
Related issue: #682
In addition, after our testing, this version can be compatible with the yaml format of the previous version.
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.
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
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
@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.
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
@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.
@narugo1992 Did you by any chance publish your fork anywhere?
I'm using responses for testing with an external API which recently switched one of their endpoints from xml text to zip archive. Would be great to try this out, hopefully without needing to learn how to build it myself :sweat_smile:
