Added param response_process (callback) to process response object

[sohang3112]

Requests to some urls return binary data which cannot be decoded (eg. image data).
In these cases, param parse_bytes can be set to False.

[sohang3112]

@singhsidhukuldeep Please review my pull request, and let me know if something can be improved. Thanks in advance 🙂

[singhsidhukuldeep]

Please resolve conflicts

[sohang3112]

@singhsidhukuldeep Resolved the conflicts - they were because of the commits in main branch since February.

[sohang3112]

@singhsidhukuldeep Did you have a chance to review the PR yet?

[singhsidhukuldeep]

I don't think there is any need for this, you can set parse_json as False

This will give you the raw response and later you can process it as you want

[sohang3112]

you can set parse_json as False

@singhsidhukuldeep Setting parse_json to False does not help, because the request bytes are always decoded irrespective of parse_json. In the following code from __init__.py:

data = response.read()
encoding = response.info().get_content_charset("utf-8")
decoded_data = data.decode(encoding)                      # THE BYTES ARE ALWAYS DECODED TO STRING FIRST
self.results[loc] = (
    json.loads(
        decoded_data) if self.parse_json else decoded_data
)
self.queue.task_done()

Decoding the bytes is always done (data.decode(encoding)). This is not desirable for binary files like images, videos, etc.

[singhsidhukuldeep]

Makes sense, let's make it parse JSON or parse nothing and return the Request object itself

[sohang3112]

Makes sense, let's make it parse JSON or parse nothing and return the Request object itself

That sounds good. But this will break compatibility for existing users - when parse_json=False, they would expect the response text, but would instead get the response object.

[singhsidhukuldeep]

@sohang3112 need to rethink this; people will start adding specific post processes for their own use cases

We need to keep it light-weight and extendable

one idea is to allow the user to pass their own post-processing logic as a callable function (post_process_func), We will pass the request object through that, this can overwrite parse_json when not None

[sohang3112]

@singhsidhukuldeep Yes this sounds like a good idea - if a callable is passed, then pass it the request object and use its result. Otherwise do existing processing.

[sohang3112]

Sorry closed accidentally 😞
Re-opening..

[sohang3112]

if a callable is passed, then pass it the request object and use its result. Otherwise do existing processing.

@singhsidhukuldeep Should I modify my PR to do this??

[singhsidhukuldeep]

Yes please

…

On Tue, 12 Sept 2023, 4:11 pm Sohang Chopra, ***@***.***> wrote: if a callable is passed, then pass it the request object and use its result. Otherwise do existing processing. @singhsidhukuldeep </~https://github.com/singhsidhukuldeep> Should I modify my PR to do this?? — Reply to this email directly, view it on GitHub <#10 (comment)>, or unsubscribe </~https://github.com/notifications/unsubscribe-auth/ACOBEA4PXPSSAGTLFVJWJOLX2A36PANCNFSM6AAAAAAU4Q2YRI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[sohang3112]

@singhsidhukuldeep I updated the PR to accept optional callback response_process which is used like this:

response_ok, data = response_process(response)

data is used if response_ok is true, otherwise request will be retried.

[sohang3112]

@singhsidhukuldeep Please review and merge my PR. Let me know if any further changes are needed.

[sohang3112]

@singhsidhukuldeep Please merge my PR. Let me know if any further changes are required from my side.

[sohang3112]

@singhsidhukuldeep Did you have a chance to review the PR yet? As I mentioned previously, I had modified the PR to include the changes we discussed.

[sohang3112]

@singhsidhukuldeep Hi. Did you have a look at the PR yet? As I mentioned before, I modified the PR to include the changes we discussed.

[singhsidhukuldeep]

Hi @sohang3112
I am occupied with few things, will close this shortly

[sohang3112]

@singhsidhukuldeep Please review & merge this PR now.

[sohang3112]

@singhsidhukuldeep Hi. Please review & merge this PR.

[sohang3112]

@singhsidhukuldeep Any update?? Please review & merge this PR.