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

[HttpClient] Document pre-conditions for HttpClient asynchronous behavior #13078

Closed
wants to merge 1 commit into from

Conversation

@B-Galati
Copy link
Contributor

B-Galati commented Feb 8, 2020

@B-Galati B-Galati force-pushed the B-Galati:http-async-clarification branch from a40d6ab to ab7969c Feb 8, 2020
@B-Galati B-Galati changed the title Document pre-conditions for HttpClient asynchronous behavior [HttpClient] Document pre-conditions for HttpClient asynchronous behavior Feb 8, 2020
@@ -116,6 +116,14 @@ immediately instead of waiting to receive the response::
This component also supports :ref:`streaming responses <http-client-streaming-responses>`
for full asynchronous applications.

.. caution::

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Feb 11, 2020

Member

this is too early and too scary in the page :)
I would try to make the description more... descriptive.
We feel the stress when reading it, but there is nothing special here, this is normal operations. It needs to be better described of course.

This comment has been minimized.

Copy link
@B-Galati

B-Galati Feb 11, 2020

Author Contributor

Agree for the scary thing :D

Where do you think it would fit the best in the doc?
IMHO it should be mentioned ASAP.

This comment has been minimized.

Copy link
@OskarStark

OskarStark Feb 11, 2020

Contributor

What about adding a Troubleshooting headline + a subsection and mentioning not just the solution but also the WHY you came to this point?

@B-Galati

This comment has been minimized.

Copy link
Contributor Author

B-Galati commented Feb 11, 2020

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 11, 2020

Troubleshooting might make sense to me as there are other behaviors that ppl didn't expect until I explained them on issues (e.g. some exceptions thrown when using for each.)

This section could collect those explanations.

(async works also with the native client, and with the amp one soon.)

@B-Galati

This comment has been minimized.

Copy link
Contributor Author

B-Galati commented Feb 11, 2020

Troubleshooting might make sense to me as there are other behaviors that ppl didn't expect until I explained them on issues (e.g. some exceptions thrown when using for each.)

I would be glad to add them all in this PR. Would you have issue links that I could use?

(async works also with the native client, and with the amp one soon.)

Awesome!

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 14, 2020

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Feb 14, 2020

Thanks, replaced by #13136

@B-Galati B-Galati deleted the B-Galati:http-async-clarification branch Feb 15, 2020
javiereguiluz added a commit that referenced this pull request Feb 15, 2020
…(nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] better explain how to deal with exceptions

Replaces #13078 and #13134

Commits
-------

61cddf5 [HttpClient] better explain how to deal with exceptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.