Add documentation to Microsoft.Extensions.Http by resistr · Pull Request #57719 · dotnet/runtime

resistr · 2021-08-19T09:44:36Z

Items Addressed:

M:Microsoft.Extensions.Http.HttpMessageHandlerBuilder.CreateHandlerPipeline
(System.Net.Http.HttpMessageHandler,System.Collections.Generic.IEnumerable{System.Net.Http.DelegatingHandler})
M:Microsoft.Extensions.Http.IHttpMessageHandlerBuilderFilter.Configure
(System.Action{Microsoft.Extensions.Http.HttpMessageHandlerBuilder})

Microsoft.Extensions.Http.Logging:

M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.#ctor(Microsoft.Extensions.Logging.ILogger)
M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.#ctor
(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Http.HttpClientFactoryOptions)
M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.SendAsync
(System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken)
M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.#ctor(Microsoft.Extensions.Logging.ILogger)
M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.#ctor
(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Http.HttpClientFactoryOptions)
M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.SendAsync
(System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken)
T:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler
T:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler

Contributes to #43916

- M:Microsoft.Extensions.Http.HttpMessageHandlerBuilder.CreateHandlerPipeline (System.Net.Http.HttpMessageHandler,System.Collections.Generic.IEnumerable{System.Net.Http.DelegatingHandler}) - M:Microsoft.Extensions.Http.IHttpMessageHandlerBuilderFilter.Configure (System.Action{Microsoft.Extensions.Http.HttpMessageHandlerBuilder}) - M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.#ctor(Microsoft.Extensions.Logging.ILogger) - M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.#ctor (Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Http.HttpClientFactoryOptions) - M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.SendAsync (System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken) - M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.#ctor(Microsoft.Extensions.Logging.ILogger) - M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.#ctor (Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Http.HttpClientFactoryOptions) - M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.SendAsync (System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken) - T:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler - T:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler Fix dotnet#43916

ghost · 2021-08-19T09:44:43Z

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Items Addressed:

M:Microsoft.Extensions.Http.HttpMessageHandlerBuilder.CreateHandlerPipeline
(System.Net.Http.HttpMessageHandler,System.Collections.Generic.IEnumerable{System.Net.Http.DelegatingHandler})
M:Microsoft.Extensions.Http.IHttpMessageHandlerBuilderFilter.Configure
(System.Action{Microsoft.Extensions.Http.HttpMessageHandlerBuilder})
M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.#ctor(Microsoft.Extensions.Logging.ILogger)
M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.#ctor
(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Http.HttpClientFactoryOptions)
M:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.SendAsync
(System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken)
M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.#ctor(Microsoft.Extensions.Logging.ILogger)
M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.#ctor
(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Http.HttpClientFactoryOptions)
M:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.SendAsync
(System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken)
T:Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler
T:Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler

Items not addressed:

Microsoft.Extensions.Http namespace
This has to be documented directly in dotnet-api-docs.
M:Microsoft.Extensions.Http.HttpClientFactoryOptions.#ctor
ctor is inferred; does not exist in code; also document directly in dotnet-api-docs? Uncertain.
M:Microsoft.Extensions.Http.HttpMessageHandlerBuilder.#ctor
ctor is inferred; does not exist in code; also document directly in dotnet-api-docs? Uncertain.
M:Microsoft.Extensions.Http.PolicyHttpMessageHandler.SendAsync
(System.Net.Http.HttpRequestMessage,System.Threading.CancellationToken)
Could not find class/method to add documentation to? Possibly removed?
Microsoft.Extensions.Http.Logging namespace
This has to be documented directly in dotnet-api-docs.

Fix #43916

Author:	resistr
Assignees:	-
Labels:	`area-Extensions-HttpClientFactory`
Milestone:	-

dnfadmin · 2021-08-19T09:44:50Z

All CLA requirements met.

resistr · 2021-08-19T09:46:12Z

@carlossanlop Apologies I failed to add you to this as requested.

karelz · 2021-08-19T10:02:17Z

@resistr thanks a lot for your PR!

I updated top post and changed "Fixes" to "Contributes to" as it does not address all items in the issue linked. I also deleted the list of APIs which are not covered by this PR from the top post. Hopefully I didn't mess up things.

src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs

…pMessageHandler.cs Co-authored-by: Martin Vseticka <203266+MartyIX@users.noreply.github.com>

CarnaViire

Thanks for the contribution! I left some comments on what I believe will be more accurate to say.

CarnaViire · 2021-08-19T14:42:53Z

src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandlerBuilder.cs

        public abstract HttpMessageHandler Build();

+        /// <summary>
+        /// Creates an instance of an <see cref="HttpMessageHandler"/> using the <see cref="DelegatingHandler"/> instances


As it doesn't create a new instance, but only links existing ones, I believe it should say something like
Constructs an instance of <see cref="HttpMessageHandler"/> by chaining <paramref name="additionalHandlers"/> one after another with <paramref name="primaryHandler"/> in the end of the chain.
(let's also leave the types of params, e.g. the fact it's DelegatingHandler to <param> part of docs)

I've made the recommended changes and thanks for the feedback.
@resistr

CarnaViire · 2021-08-19T14:53:43Z

src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandlerBuilder.cs


+        /// <summary>
+        /// Creates an instance of an <see cref="HttpMessageHandler"/> using the <see cref="DelegatingHandler"/> instances
+        /// provided by <paramref name="additionalHandlers"/>. The resulting pipeline can be used to manually create <see cref="HttpClient"/>


I don't think this method is supposed/expected to be used outside of HttpClientFactory infrastructure, especially with it being protected, so I don't think "manual creating" applies here... I'd say it's more like
The resulting pipeline is used by <see cref="IHttpClientFactory"/> infrastructure to create <see cref="HttpClient"/> instances with customized message handlers. The resulting pipeline can also be accessed by using <see cref="IHttpMessageHandlerFactory"/> instead of <see cref="IHttpClientFactory"/>.

Surry for all of the noise in this section; I had borrowed the text from https://github.com/aspnet/AspNetWebStack/blob/master/src/System.Net.Http.Formatting/HttpClientFactory.cs#L58 referenced in the method.

I've made the recommended changes and thanks for the feedback.

src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandlerBuilder.cs

src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingScopeHttpMessageHandler.cs

…peHttpMessageHandler.cs Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

…Builder.cs Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

Reworded the cstor documentation on HttpMessageHandlerBuilder based on feedback from active PR. Contributes to dotnet#43916

CarnaViire

LGTM, thanks!

CarnaViire · 2021-08-19T16:46:46Z

@carlossanlop adding you as a reviewer as you requested in #43916

src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingScopeHttpMessageHandler.cs

src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs

src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandlerBuilder.cs

…Builder.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

…pMessageHandler.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

…peHttpMessageHandler.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

Augment documentation for SendAsync in LoggingScopeHttpMessageHandler and LoggingHttpMessageHandler Contributes to dotnet#43916

CarnaViire · 2021-08-25T19:14:19Z

@carlossanlop could you pls check out replies to your comments and let us know your thoughts?

carlossanlop

LGTM. Thank you so much for your contribution, @resistr!

ghost added area-Extensions-HttpClientFactory community-contribution Indicates that the PR has been added by a community member labels Aug 19, 2021

karelz assigned CarnaViire Aug 19, 2021

karelz requested a review from CarnaViire August 19, 2021 10:00

karelz mentioned this pull request Aug 19, 2021

API documentation debt - Microsoft.Extensions.Http #43916

Closed

MartyIX reviewed Aug 19, 2021

View reviewed changes

src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs Outdated Show resolved Hide resolved

Update src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHtt…

8ee45f0

…pMessageHandler.cs Co-authored-by: Martin Vseticka <203266+MartyIX@users.noreply.github.com>

CarnaViire requested changes Aug 19, 2021

View reviewed changes

resistr and others added 4 commits August 19, 2021 10:22

Update src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingSco…

6bd93a5

…peHttpMessageHandler.cs Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

Update src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandler…

89caca2

…Builder.cs Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

Update src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandler…

68c01c4

…Builder.cs Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

Update to HttpMessageHandlerBuilder cstor doc

44a2e36

Reworded the cstor documentation on HttpMessageHandlerBuilder based on feedback from active PR. Contributes to dotnet#43916

CarnaViire approved these changes Aug 19, 2021

View reviewed changes

CarnaViire requested a review from carlossanlop August 19, 2021 16:45

carlossanlop reviewed Aug 20, 2021

View reviewed changes

karelz added this to the 7.0.0 milestone Aug 20, 2021

resistr and others added 6 commits August 20, 2021 05:20

Update src/libraries/Microsoft.Extensions.Http/src/HttpMessageHandler…

f900384

…Builder.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

Update src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHtt…

19abb18

…pMessageHandler.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

Update src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHtt…

29b7ce0

…pMessageHandler.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

Update src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingSco…

833a96b

…peHttpMessageHandler.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

Update src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingSco…

cb978c8

…peHttpMessageHandler.cs Co-authored-by: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com>

Update documentation to Microsoft.Extensions.Http

0a1fc71

Augment documentation for SendAsync in LoggingScopeHttpMessageHandler and LoggingHttpMessageHandler Contributes to dotnet#43916

carlossanlop approved these changes Aug 31, 2021

View reviewed changes

CarnaViire merged commit 5abee29 into dotnet:main Aug 31, 2021

CarnaViire mentioned this pull request Sep 3, 2021

Port triple-slash comments to docs for Microsoft.Extensions.Http dotnet/dotnet-api-docs#7103

Merged

ghost locked as resolved and limited conversation to collaborators Sep 30, 2021

Conversation

resistr commented Aug 19, 2021 • edited by karelz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Aug 19, 2021

Uh oh!

dnfadmin commented Aug 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

resistr commented Aug 19, 2021

Uh oh!

karelz commented Aug 19, 2021

Uh oh!

Uh oh!

CarnaViire left a comment

Choose a reason for hiding this comment

Uh oh!

CarnaViire Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

resistr Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

CarnaViire Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

resistr Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CarnaViire left a comment

Choose a reason for hiding this comment

Uh oh!

CarnaViire commented Aug 19, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CarnaViire commented Aug 25, 2021

Uh oh!

carlossanlop left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

resistr commented Aug 19, 2021 •

edited by karelz

Loading

dnfadmin commented Aug 19, 2021 •

edited

Loading