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

Support for asynchronous request data #92

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

egirshov
Copy link

@egirshov egirshov commented Jun 3, 2013

I'm targeting use case when body is not fully available at the time of making request. Adding new option 'stream_request' (should be used together with 'stream_to' for response)

@cmullaparthi
Copy link
Owner

Hi, have you looked at whether you can use the option to supply a fun for the Body? That fun can be used to generate the body after a request is submitted.

@egirshov
Copy link
Author

egirshov commented Jun 4, 2013

Yes, I did. I found this way more convenient, but if you believe there is no need to have anything besides fun as a body, I would not argue. 125fe14 or a similar fix for tracing could be still considered.

@cmullaparthi
Copy link
Owner

I've read the patch more closely. It looks good and I like it. Thank you. A couple of questions/suggestions if I may.

  • Line 984 can probably be changed to
get_value(stream_to, Options, undefined) =/= undefined.

rather than defining a new option?

  • What happens if the calling process never calls ibrowse:send_done/1 ?

@egirshov
Copy link
Author

egirshov commented Jun 4, 2013

I introduced new option deliberatly to make it possible in the future for both request and response to be sent complete or in chunks. Otherwise, if we want 'streaming request' to imply 'streaming response' I could change line 984 as you suggested (but then use for example 'undefined' value for the Body as an indication of send_chunk/send_done interface).

Regarding timeouts - currently if no send_done is called ever, request will fail with ibrowse_async_response_timeout (when corresponding timeout expires).

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.

2 participants