Convert to net8 on release 1.13

[pngan]

Description

Converted quickstart projects to use .Net 8 runtime and SDK in all C# projects.

Issue reference

Please reference the issue this PR will close: #980

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• The quickstart code compiles correctly
• You've tested new builds of the quickstart if you changed quickstart code
• You've updated the quickstart's README if necessary
• If you have changed the steps for a quickstart be sure that you have updated the automated validation accordingly. All of our quickstarts have annotations that allow them to be executed automatically as code. For more information see mechanical-markdown. For user guide with examples see Examples.

[pngan]

@paulyuk I've created a new pull request rather than re-use the original pull request (#960 ) that you reviewed recently. This is because the changes are now based off branch release-1.13 rather than master, and I thought it would be tidier to start with a fresh pr.

I've incorporated the changes you suggested in pull request #960. Also, I realized that the batch project uses the Swashbuckle libraries which have not been updated to .Net 8. I've instead reverted this project to be based on .Net 6 to keep it consistent with all of it's other dependencies.

[paulyuk]

Hi - looks really close, just it still targets the dapr:master base instead of the dapr:release-1.13. Could you please edit the PR and toggle branch to release-1.13? I also approved checks/tests to run to see how this looks in CI. thank you for all your efforts! looking really close

Also if you have time we can fully upgrade Bindings(batch) project for .net 8. But if not this PR is still great step.

[paulyuk]

Here to make life easier I edited base to be release-1.13 for you.

[paulyuk]

@pngan I spotted the issue why Validate C# test is failing. It looks like the console output changes slightly using this combination of runtime and dapr.

Please change the following Line 37 in your Actors Readme from:

  - "Request finished HTTP/1.1 GET http://127.0.0.1:5001/healthz - - - 200"

to:

  - "Request finished HTTP/1.1 GET http://127.0.0.1:5001/healthz - 200"

Notice it just removes two dashes and spaces before the 200.

Here's a trick I do to validate tests quickly on the local machine before you commit and try the long 30m+ CI/CD tests. You can test in your own actors/csharp/sdk folder (or any quickstart folder with a makefile) using make validate to see the error turns into all 200s. If you get an error that you're missing Mechanical Markdown, do this first:

sudo pip3 install mechanical-markdown

hth, and let me know if you need help.

[philliphoff]

Also, I realized that the batch project uses the Swashbuckle libraries which have not been updated to .Net 8. I've instead reverted this project to be based on .Net 6 to keep it consistent with all of it's other dependencies.

@pngan It appears that some projects might reference the Swashbuckle libraries but not actually use them. If that's preventing moving the project to .NET 8, I'd recommend just removing the Swashbuckle references.

[paulyuk]

Hey @pngan just wondering if you have some time to look at this again. I'd love to get it in for this release. I can also help.

[pngan]

Hi @paulyuk Thanks for rebasing the pull request on release-1.13. If that was the remaining issue, do you think this pull request is ready to be merged?

I notice that a C# pull request check is failing a /healthz end-point check. The debugging output would suggest that that endpoint is completing fine. Is this something you could help with?

[pngan]

@philliphoff I'd rather leave the upgrade to .net 8 for the binding project to someone else. Could we make a github issue specifically targeted to the upgrade of binding project? While the swagger library may no longer be referenced, it seems that the project may be in an indeterminate state if in the past there was OpenApi documentation being auto-generated - perhaps there is some work to restore the OpenApi generation. But this is something I don't want to sign up for at this time.

[pngan]

As a general comment about updating to .net 8 - I noticed words on the dapr public website references .NET 6+ . This should be updated to say .NET 8+. I've have not managed to locate those strings in this github repo, so presumably that content is stored elsewhere. We should update those instructions, in order for it to stay in sync with these changes (once release-1.13 is released!)

[paulyuk]

Hi @paulyuk Thanks for rebasing the pull request on release-1.13. If that was the remaining issue, do you think this pull request is ready to be merged?

I notice that a C# pull request check is failing a /healthz end-point check. The debugging output would suggest that that endpoint is completing fine. Is this something you could help with?

Hi @pngan. Good point. The /healthz endpoint is working well as you point out. The issue with the test is simply that the output of the check now returns a string with less dashs. The "fix" to this is to change Line 37 of the Readme to the new string, which is:

  - "Request finished HTTP/1.1 GET http://127.0.0.1:5001/healthz - 200"

If you want to make that change and validate the check works, that's great. I can also help do this I think by contributing to your PR. Fix is available in this PR: pngan#1

[paulyuk]

As a general comment about updating to .net 8 - I noticed words on the dapr public website references .NET 6+ . This should be updated to say .NET 8+. I've have not managed to locate those strings in this github repo, so presumably that content is stored elsewhere. We should update those instructions, in order for it to stay in sync with these changes (once release-1.13 is released!)

Great catch. The docs have a richer copy of this readme and are maintained by @hhunter-ms and others. She knows about this PR, so we can get that updated as well. If you're curious the source for that website doc is here: https://github.com/dapr/docs/edit/v1.12/daprdocs/content/en/getting-started/quickstarts/workflow-quickstart.md

[paulyuk]

@philliphoff I'd rather leave the upgrade to .net 8 for the binding project to someone else. Could we make a github issue specifically targeted to the upgrade of binding project? While the swagger library may no longer be referenced, it seems that the project may be in an indeterminate state if in the past there was OpenApi documentation being auto-generated - perhaps there is some work to restore the OpenApi generation. But this is something I don't want to sign up for at this time.

Ok I'll take care of this.
Fix comes in this PR if you could please merge it: pngan#1

[paulyuk]

Hey @pngan -- may I suggest you review and merge a few complementary changes in this PR and/or copy the changes into your branch and commit so this PR picks it up? Then I'll be ready to sign off and merge this PR. I ran all these tests, and with our collab on both sets of commits all tests pass for all quickstarts, including Actors and Bindings where we spotted challenges.

[pngan]

@paulyuk

Here's a trick I do to validate tests quickly on the local machine before you commit and try the long 30m+ CI/CD tests. You can test in your own actors/csharp/sdk folder (or any quickstart folder with a makefile) using make validate to see the error turns into all 200s. If you get an error that you're missing Mechanical Markdown, do this first:

sudo pip3 install mechanical-markdown

hth, and let me know if you need help.

Hi Paul,

I ran the make validate in the spirit of TDD before fixing the /healthz documentation testing. Interestingly, all tests passed locally, and the test on the PR check was not picked up. I'll fix the check, but letting you know about that.

phil@z420:~/github/forked/dapr/quickstarts$ make validate
mm.py -l -s "bash -c" README.md

External link validation:
	https://github.com/dapr/quickstarts/actions?workflow=samples Status: 200
	https://discord.com/channels/778680217417809931/778680217417809934 Status: 200
	https://www.apache.org/licenses/LICENSE-2.0 Status: 200
	https://docs.dapr.io/getting-started/install-dapr-cli/ Status: 200
	https://docs.dapr.io/getting-started/quickstarts/ Status: 200
	https://github.com/dapr/community/blob/master/CODE-OF-CONDUCT.md Status: 200

[paulyuk]

@paulyuk

Here's a trick I do to validate tests quickly on the local machine before you commit and try the long 30m+ CI/CD tests. You can test in your own actors/csharp/sdk folder (or any quickstart folder with a makefile) using make validate to see the error turns into all 200s. If you get an error that you're missing Mechanical Markdown, do this first:

sudo pip3 install mechanical-markdown

hth, and let me know if you need help.

Hi Paul,

I ran the make validate in the spirit of TDD before fixing the /healthz documentation testing. Interestingly, all tests passed locally, and the test on the PR check was not picked up. I'll fix the check, but letting you know about that.

phil@z420:~/github/forked/dapr/quickstarts$ make validate
mm.py -l -s "bash -c" README.md

External link validation:
	https://github.com/dapr/quickstarts/actions?workflow=samples Status: 200
	https://discord.com/channels/778680217417809931/778680217417809934 Status: 200
	https://www.apache.org/licenses/LICENSE-2.0 Status: 200
	https://docs.dapr.io/getting-started/install-dapr-cli/ Status: 200
	https://docs.dapr.io/getting-started/quickstarts/ Status: 200
	https://github.com/dapr/community/blob/master/CODE-OF-CONDUCT.md Status: 200

Hey @pngan thanks a lot for doing the local checks. Also good call pasting in exactly what you tried. I can see you're in the root of the quickstart folder when you run make validate. Make validate is not recursive deep into the folders, so that only tested the readme in the root folder and it did not go deeper into any of the specific quickstarts like C# Actors.

Now please try make validate in your ~/github/forked/dapr/quickstarts/actors/csharp/sdk folder. I'm going to guess you will see a different result before and after the changes merge.

The GitHub actions workflows however do recursively walkthrough each makefile in each sub folder and that's why we see a bunch more output and results from each language and each quickstart.

[pngan]

@paulyuk I've merged the branch with the additional changes (I noticed you fixed the healthcheck test in this pr). We're ready to run pull request checks workflow again. The order of commits have changed, with my commits coming after the ones from your pr: this is simply to change the commit message to include the sign-off field to satisfy the DCO checks; there were no code changes in those follow on commits, beyond what was there from a few days ago.

[pngan]

This may be unrelated to the pull request, but on my local machine, the actor quickstart exercise and the actor validation fails, where the client is unable to establish a connection to the actor. I suspect this is a problem on my specific machine because this error also occurs using .NET 7 and release-1.13 for me. Be interested to see if the tests pass on github actions.

		== APP == Startup up...
		== APP == Calling SetDataAsync on SmokeDetectorActor:1...
		== APP == Got response: Success
		== APP == Calling GetDataAsync on SmokeDetectorActor:1...
		== APP == Device 1 state: Location: First Floor, Status: Ready
		== APP == Calling SetDataAsync on SmokeDetectorActor:2...
		== APP == Got response: Success
		== APP == Calling GetDataAsync on SmokeDetectorActor:2...
		== APP == Device 2 state: Location: Second Floor, Status: Ready
		== APP == Registering the IDs of both Devices...
		== APP == Registered devices: 1, 2
		== APP == Detecting smoke on Device 1...
		time="2024-02-18T16:52:29.656447457+13:00" level=info msg="Error processing operation DaprBuiltInActorRetries. Retrying in 1s…" app_id=actorclient instance=z420 scope=dapr.runtime type=log ver=1.12.2
		== APP == Unhandled exception. Dapr.DaprApiException: error invoke actor method: failed to invoke target 192.168.1.79:35937 after 3 retries. Error: rpc error: code = Unavailable desc = last connection error: connection error: desc = "transport: Error while dialing: dial tcp 192.168.1.79:35937: connect: connection refused"
		== APP ==    at Dapr.Actors.DaprHttpInteractor.SendAsyncHandleUnsuccessfulResponse(Func`1 requestFunc, String relativeUri, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.DaprHttpInteractor.SendAsync(Func`1 requestFunc, String relativeUri, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.DaprHttpInteractor.InvokeActorMethodWithRemotingAsync(ActorMessageSerializersManager serializersManager, IActorRequestMessage remotingRequestRequestMessage, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.Communication.Client.ActorRemotingClient.InvokeAsync(IActorRequestMessage remotingRequestMessage, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.Client.ActorProxy.InvokeMethodAsync(Int32 interfaceId, Int32 methodId, String methodName, IActorRequestMessageBody requestMsgBodyValue, CancellationToken cancellationToken)
		== APP ==    at SmartDevice.Program.Main(String[] args) in /home/phil/github/forked/dapr/quickstarts/actors/csharp/sdk/client/Program.cs:line 68
		== APP ==    at SmartDevice.Program.<Main>(String[] args)
		ℹ️  

[paulyuk]

This may be unrelated to the pull request, but on my local machine, the actor quickstart exercise and the actor validation fails, where the client is unable to establish a connection to the actor. I suspect this is a problem on my specific machine because this error also occurs using .NET 7 and release-1.13 for me. Be interested to see if the tests pass on github actions.

		== APP == Startup up...
		== APP == Calling SetDataAsync on SmokeDetectorActor:1...
		== APP == Got response: Success
		== APP == Calling GetDataAsync on SmokeDetectorActor:1...
		== APP == Device 1 state: Location: First Floor, Status: Ready
		== APP == Calling SetDataAsync on SmokeDetectorActor:2...
		== APP == Got response: Success
		== APP == Calling GetDataAsync on SmokeDetectorActor:2...
		== APP == Device 2 state: Location: Second Floor, Status: Ready
		== APP == Registering the IDs of both Devices...
		== APP == Registered devices: 1, 2
		== APP == Detecting smoke on Device 1...
		time="2024-02-18T16:52:29.656447457+13:00" level=info msg="Error processing operation DaprBuiltInActorRetries. Retrying in 1s…" app_id=actorclient instance=z420 scope=dapr.runtime type=log ver=1.12.2
		== APP == Unhandled exception. Dapr.DaprApiException: error invoke actor method: failed to invoke target 192.168.1.79:35937 after 3 retries. Error: rpc error: code = Unavailable desc = last connection error: connection error: desc = "transport: Error while dialing: dial tcp 192.168.1.79:35937: connect: connection refused"
		== APP ==    at Dapr.Actors.DaprHttpInteractor.SendAsyncHandleUnsuccessfulResponse(Func`1 requestFunc, String relativeUri, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.DaprHttpInteractor.SendAsync(Func`1 requestFunc, String relativeUri, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.DaprHttpInteractor.InvokeActorMethodWithRemotingAsync(ActorMessageSerializersManager serializersManager, IActorRequestMessage remotingRequestRequestMessage, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.Communication.Client.ActorRemotingClient.InvokeAsync(IActorRequestMessage remotingRequestMessage, CancellationToken cancellationToken)
		== APP ==    at Dapr.Actors.Client.ActorProxy.InvokeMethodAsync(Int32 interfaceId, Int32 methodId, String methodName, IActorRequestMessageBody requestMsgBodyValue, CancellationToken cancellationToken)
		== APP ==    at SmartDevice.Program.Main(String[] args) in /home/phil/github/forked/dapr/quickstarts/actors/csharp/sdk/client/Program.cs:line 68
		== APP ==    at SmartDevice.Program.<Main>(String[] args)
		ℹ️  

Very nice! I kicked off tests. I agree let's see how the formal CI tests do. The timing of the actors test is very picky, and sometimes running it again will show different results.

[paulyuk]

LGTM!

[paulyuk]

Just waiting on tests, and then we'll merge.

[pngan]

@paulyuk All tests have passed. You are all good to merge (I don't see the merge button)

[paulyuk]

It's great -- thank you @pngan for this important contribution! We really needed .NET 8 support!