Skip to content
This repository was archived by the owner on Apr 27, 2026. It is now read-only.

Changes to header parsing#3672

Merged
jonsequitur merged 7 commits into
dotnet:mainfrom
bleaphar:bleaphar/HeaderParsingFix
Sep 19, 2024
Merged

Changes to header parsing#3672
jonsequitur merged 7 commits into
dotnet:mainfrom
bleaphar:bleaphar/HeaderParsingFix

Conversation

@bleaphar

Copy link
Copy Markdown
Contributor

This PR fixes the issue with trying to get values from the headers of an http response. It looks to see if the user has a path of four, assuming they want the first element in the list and if there's a path of five then it gets the first element in the list that matches the value of the string. Additionally, a test for this scenario is included.

Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Co-authored-by: Jon Sequeira <jonsequeira@gmail.com>
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated

@shyamnamboodiripad shyamnamboodiripad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm modulo comments

Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.cs Outdated

var returnValue = secondResult.Events.OfType<ReturnValueProduced>().First();

var response = (HttpResponse)returnValue.Value;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't looks like returnValue or response are needed.


var returnValue = secondResult.Events.OfType<ReturnValueProduced>().First();

var response = (HttpResponse)returnValue.Value;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't looks like returnValue or response are needed.

Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.NamedRequest.cs Outdated
{
var response = new HttpResponseMessage(HttpStatusCode.OK);
response.RequestMessage = message;
var contentString = @"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a raw string (""") will improve readability.

Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.NamedRequest.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.NamedRequest.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http.Tests/HttpKernelTests.NamedRequest.cs Outdated
Comment thread src/Microsoft.DotNet.Interactive.Http/HttpKernel.cs Outdated
@jonsequitur
jonsequitur enabled auto-merge (squash) September 19, 2024 17:55
@jonsequitur
jonsequitur merged commit 3dc0d11 into dotnet:main Sep 19, 2024
@colombod colombod added the Area-HTTP HttpKernel, Visual Studio HTTP Editor label Oct 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Area-HTTP HttpKernel, Visual Studio HTTP Editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants