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

Update giraffe sample to .NET 6 alpha #6891

Merged
merged 3 commits into from
Nov 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions frameworks/FSharp/giraffe/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ This application tests Giraffe in 3 modes:

**Platforms**

* .NET 5 (Windows and Linux)
* .NET 6 (Windows and Linux)

**Web Servers**

Expand All @@ -29,7 +29,7 @@ This application tests Giraffe in 3 modes:

All source code is inside `Program.fs`.

App listens for a signle command line argument to pick the desired JSON implementation:
App listens for a single command line argument to pick the desired JSON implementation:

- `system`: `System.Text.Json`
- `utf8`: `Utf8Json`
Expand Down
4 changes: 2 additions & 2 deletions frameworks/FSharp/giraffe/giraffe-newtonsoft.dockerfile
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
FROM mcr.microsoft.com/dotnet/sdk:5.0 AS build
FROM mcr.microsoft.com/dotnet/sdk:6.0 AS build
WORKDIR /app
COPY src/App .
RUN dotnet publish -c Release -o out

FROM mcr.microsoft.com/dotnet/aspnet:5.0 AS runtime
FROM mcr.microsoft.com/dotnet/aspnet:6.0 AS runtime
ENV ASPNETCORE_URLS http://+:8080
WORKDIR /app
COPY --from=build /app/out ./
Expand Down
4 changes: 2 additions & 2 deletions frameworks/FSharp/giraffe/giraffe-utf8json.dockerfile
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
FROM mcr.microsoft.com/dotnet/sdk:5.0 AS build
FROM mcr.microsoft.com/dotnet/sdk:6.0 AS build
WORKDIR /app
COPY src/App .
RUN dotnet publish -c Release -o out

FROM mcr.microsoft.com/dotnet/aspnet:5.0 AS runtime
FROM mcr.microsoft.com/dotnet/aspnet:6.0 AS runtime
ENV ASPNETCORE_URLS http://+:8080
WORKDIR /app
COPY --from=build /app/out ./
Expand Down
4 changes: 2 additions & 2 deletions frameworks/FSharp/giraffe/giraffe.dockerfile
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
FROM mcr.microsoft.com/dotnet/sdk:5.0 AS build
FROM mcr.microsoft.com/dotnet/sdk:6.0 AS build
WORKDIR /app
COPY src/App .
RUN dotnet publish -c Release -o out

FROM mcr.microsoft.com/dotnet/aspnet:5.0 AS runtime
FROM mcr.microsoft.com/dotnet/aspnet:6.0 AS runtime
ENV ASPNETCORE_URLS http://+:8080
WORKDIR /app
COPY --from=build /app/out ./
Expand Down
10 changes: 5 additions & 5 deletions frameworks/FSharp/giraffe/src/App/App.fsproj
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
<TargetFramework>net6.0</TargetFramework>
<EnableDefaultContentItems>false</EnableDefaultContentItems>
</PropertyGroup>

<ItemGroup>
<PackageReference Update="FSharp.Core" Version="6.0.0" />
<PackageReference Include="Dapper" Version="2.0.90" />
<PackageReference Include="Giraffe" Version="5.0.0" />
<PackageReference Include="Npgsql" Version="5.0.7" />
<PackageReference Update="FSharp.Core" Version="6.0.1" />
<PackageReference Include="Dapper" Version="2.0.123" />
<PackageReference Include="Giraffe" Version="6.0.0-alpha-1" />
<PackageReference Include="Npgsql" Version="6.0.0" />
</ItemGroup>

<ItemGroup>
Expand Down
41 changes: 24 additions & 17 deletions frameworks/FSharp/giraffe/src/App/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,16 @@ module HttpHandlers =
route "/fortunes" fortunes
]


module Main =
open Microsoft.AspNetCore.Builder
open Microsoft.AspNetCore.Hosting
open Microsoft.Extensions.DependencyInjection
open Giraffe
open Giraffe.EndpointRouting
open Microsoft.Extensions.Hosting

[<EntryPoint>]
[<EntryPoint>]
let main args =
let jsonMode =
match args with
Expand All @@ -128,20 +130,25 @@ module Main =
NewtonsoftJson.Serializer(NewtonsoftJson.Serializer.DefaultSettings)
:> Json.ISerializer

WebHostBuilder()
.UseKestrel()
.Configure(
fun builder ->
builder
.UseRouting()
.UseGiraffe HttpHandlers.endpoints |> ignore)
.ConfigureServices(
fun services ->
services
.AddRouting()
.AddGiraffe()
.AddSingleton(jsonSerializer)
|> ignore)
.Build()
.Run()
let configureApp (appBuilder : IApplicationBuilder) =
Copy link

Choose a reason for hiding this comment

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

Glancing at this, is there anything here that could cause a performance change? UseKestrel being dropped? Anything else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK that's how hosting is initialized in asp.net core 6. /cc @davidfowl

Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a bit more minimal overall:

let builder = WebApplication.CreateBuilder(args)

builder.Services.AddGiraffe()
                .AddSingleton(jsonSerializer) |> ignore

let app = builder.Build()

app.UseGiraffe HttpHandlers.endpoints |> ignore

app.Run()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is another PR coming that disables logging. Could this be a change to asp.net core 5 that affected performance? Was logging enabled before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And thanks for the help!

Copy link
Contributor

@slang25 slang25 Nov 17, 2021

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@davidfowl AddRouting could indeed go but removing UseRouting errors with:

Unhandled exception. System.InvalidOperationException: EndpointRoutingMiddleware matches endpoints setup by EndpointMiddleware and so must be added to the request execution pipeline before EndpointMiddleware. Please add EndpointRoutingMiddleware by calling 'IApplicationBuilder.UseRouting' inside the call to 'Configure(...)' in the application startup code.

Are we expected to add this inside the UseGiraffe implementation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to MapGiraffeEndpoints instead of UseGiraffe

cc @dustinmoris

Copy link
Contributor

Choose a reason for hiding this comment

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

When the EndpointRouting namespace is open then the UseGiraffe extension method is doing the correct thing already.

appBuilder
.UseRouting()
.UseGiraffe HttpHandlers.endpoints
|> ignore

let configureServices (services : IServiceCollection) =
services
.AddRouting()
.AddGiraffe()
.AddSingleton(jsonSerializer)
|> ignore

let builder = WebApplication.CreateBuilder(args)
configureServices builder.Services

let app = builder.Build()

configureApp app
app.Run()

0