Skip to content

Added support for .net 8 - DTF Service Fabric #1205

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
75 changes: 45 additions & 30 deletions Directory.Packages.props
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a bunch of package changes here that will impact all projects in this repo. Have you verified what the impact to other packages are? Keep in mind we cannot have any major version revs of any transitive dependency. This is considered a breaking change to customers and we would need to also major version rev.

Copy link
Author

Choose a reason for hiding this comment

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

None of the other project's target .net8. All projects continue to be on the same version. Updated the Packages.props to a more understandable structure.

Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,26 @@
<PackageVersion Include="Azure.Storage.Blobs" Version="12.22.1" />
<PackageVersion Include="Azure.Storage.Queues" Version="12.20.0" />
<PackageVersion Include="Castle.Core" Version="5.0.0" />
<PackageVersion Include="ImpromptuInterface" Version="6.2.2" Condition="'$(TargetFramework)' == 'net462' OR '$(TargetFramework)' == 'net472'" />
<PackageVersion Include="ImpromptuInterface" Version="7.0.1" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
<PackageVersion Include="ImpromptuInterface" Version="7.0.1" Condition="'$(TargetFramework)' == 'netstandard2.0' OR '$(TargetFramework)' == 'net8.0'" />
<PackageVersion Include="Microsoft.ApplicationInsights" Version="2.21.0" />
<PackageVersion Include="Microsoft.AspNet.WebApi.OwinSelfHost" Version="5.2.6" />
<PackageVersion Include="Microsoft.AspNet.WebApi.Client" Version="5.2.6" />
<PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="6.0.0" />
<PackageVersion Include="Microsoft.CSharp" Version="4.7.0" />
<PackageVersion Include="Microsoft.Data.Services.Client" Version="5.8.5" />
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="2.1.1" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.1" />
<PackageVersion Include="Microsoft.ServiceFabric" Version="6.4.617" />
<PackageVersion Include="Microsoft.ServiceFabric.Data" Version="3.3.617" />
<PackageVersion Include="Microsoft.ServiceFabric.Services" Version="3.3.617" />
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="2.1.1" Condition="'$(TargetFramework)' != 'net8.0'" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.1" Condition="'$(TargetFramework)' != 'net8.0'" />
<PackageVersion Include="Microsoft.Extensions.Logging" Version="6.0.0" Condition="'$(TargetFramework)' != 'net8.0'" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="6.0.0" Condition="'$(TargetFramework)' != 'net8.0'" />
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried about all these != 'net8.0' checks, and how we will maintain these if/when we need to move to net10 in the future. Is there another way we can refactor this to make it easier to make future changes?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

@jviau - Can you please elaborate? I din't follow. Do you mean using IsTargetFrameworkCompatible?

Condition="!$([MSBuild]::IsTargetFrameworkCompatible('net8.0', '$(TargetFramework)'))"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes IsTargetFrameworkCompatible might be what you want. But I can't remember the exact behavior/syntax off the top of my head. But you should be able to use it to effectively do a >= net8.0

<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="1.0.0" />
<PackageVersion Include="Newtonsoft.Json" Version="13.0.1" />
<PackageVersion Include="System.Collections.Immutable" Version="1.2.0" />
<PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
<PackageVersion Include="System.Collections.Immutable" Version="1.3.1" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="6.0.1" />
<PackageVersion Include="System.Linq.Async" Version="6.0.1" />
<PackageVersion Include="System.Reactive.Compatibility" Version="4.4.1" />
<PackageVersion Include="System.Reactive.Core" Version="4.4.1" />
</ItemGroup>
<!-- Product dependencies with net462 only-->

<!-- Product dependencies with net462 only -->
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<PackageVersion Include="Microsoft.Azure.KeyVault.Core" Version="1.0.0" />
<PackageVersion Include="Microsoft.Data.Edm" Version="5.8.5" />
Expand All @@ -45,7 +43,31 @@
<PackageVersion Include="System.Spatial" Version="5.8.5" />
<PackageVersion Include="WindowsAzure.ServiceBus" Version="4.1.3" />
</ItemGroup>


<!-- Product dependencies with net462 or net472 only -->
<ItemGroup Condition="'$(TargetFramework)' == 'net462' OR '$(TargetFramework)' == 'net472'">
<PackageVersion Include="Microsoft.AspNet.WebApi.OwinSelfHost" Version="5.2.6" />
<PackageVersion Include="Microsoft.ServiceFabric" Version="6.4.617" />
<PackageVersion Include="Microsoft.ServiceFabric.Data" Version="3.3.617" />
<PackageVersion Include="Microsoft.ServiceFabric.Services" Version="3.3.617" />
<PackageVersion Include="Microsoft.ServiceFabric.AspNetCore.Kestrel" Version="3.3.617" />
<PackageVersion Include="ImpromptuInterface" Version="6.2.2" />
</ItemGroup>

<!-- Product dependencies with net8.0 -->
<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<PackageVersion Include="Microsoft.ServiceFabric" Version="10.1.2175" />
<PackageVersion Include="Microsoft.ServiceFabric.Data" Version="7.1.2175" />
<PackageVersion Include="Microsoft.ServiceFabric.Services" Version="7.1.2175" />
<PackageVersion Include="Microsoft.ServiceFabric.AspNetCore.Kestrel" Version="7.1.2175" />
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" />
<PackageVersion Include="Microsoft.Extensions.Logging" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
<PackageVersion Include="Microsoft.AspNetCore.Mvc" Version="2.3.0" />
<PackageVersion Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="8.0.15" />
</ItemGroup>

<!-- Test dependencies -->
<ItemGroup>
<PackageVersion Include="Azure.Monitor.OpenTelemetry.Exporter" Version="1.0.0-beta.4" />
Expand All @@ -54,8 +76,6 @@
<PackageVersion Include="EnterpriseLibrary.SemanticLogging.EventSourceAnalyzer.NetCore" Version="2.0.1406.4" />
<PackageVersion Include="Microsoft.AspNet.WebApi.Core" Version="5.2.6" />
<PackageVersion Include="Microsoft.Diagnostics.EventFlow.Core" Version="1.5.6" />
<PackageVersion Include="Microsoft.Extensions.Logging" Version="6.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="6.0.0" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
<PackageVersion Include="Moq" Version="4.10.0" />
<PackageVersion Include="MSTest.TestAdapter" Version="3.5.0" />
Expand All @@ -68,25 +88,24 @@
<PackageVersion Include="WindowsAzure.Storage" Version="8.7.0" Condition="'$(TargetFramework)' == 'net462'" />
</ItemGroup>

<!-- Test dependencies with netcoreapp3.1-->
<!-- Test dependencies with netcoreapp3.1 -->
<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1'">
<PackageVersion Include="EnterpriseLibrary.SemanticLogging.NetCore" Version="2.0.1406.4" />
<PackageVersion Include="System.Runtime.CompilerServices.Unsafe" Version="6.0.0" />
</ItemGroup>

<!-- Test dependencies with net472-->
<!-- Test dependencies with net472 -->
<ItemGroup Condition="'$(TargetFramework)' == 'net472'">
<PackageVersion Include="Microsoft.AspNet.WebApi.Client" Version="5.2.6" />
<PackageVersion Include="Microsoft.AspNet.WebApi.Owin" Version="5.2.6" />
<PackageVersion Include="Microsoft.Owin.Hosting" Version="4.0.0" />
</ItemGroup>

<!-- Test dependencies with net462-->
<!-- Test dependencies with net462 -->
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<PackageVersion Include="EnterpriseLibrary.SemanticLogging" version="2.0.1406.1" />
<PackageVersion Include="EnterpriseLibrary.SemanticLogging.TextFile" version="2.0.1406.1" />
<PackageVersion Include="EnterpriseLibrary.SemanticLogging" Version="2.0.1406.1" />
<PackageVersion Include="EnterpriseLibrary.SemanticLogging.TextFile" Version="2.0.1406.1" />
<PackageVersion Include="Microsoft.Owin.Hosting" Version="4.0.0" />
<PackageVersion Include="Microsoft.Tpl.Dataflow" version="4.5.24" />
<PackageVersion Include="Microsoft.Tpl.Dataflow" Version="4.5.24" />
</ItemGroup>

<!-- Samples dependencies -->
Expand All @@ -104,16 +123,12 @@
<PackageVersion Include="Vio.DurableTask.Hosting" Version="2.2.1" />
</ItemGroup>

<!-- Samples dependencies with net8.0-->
<!-- Samples dependencies with net8.0 -->
<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<PackageVersion Include="Microsoft.Extensions.Logging" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
</ItemGroup>

<!-- Samples dependencies with net462-->
<!-- Samples dependencies with net462 -->
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<PackageVersion Include="ncrontab" version="1.0.0" />
<PackageVersion Include="ncrontab" Version="1.0.0" />
</ItemGroup>

</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory),DurableTask.sln))\tools\DurableTask.props" />

<PropertyGroup>
<TargetFrameworks>net462;net472</TargetFrameworks>
<TargetFrameworks>net8.0;net462;net472</TargetFrameworks>
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes look fairly extensive, especially adding a new TFM. Have you evaluated what version change needs to happen? if there are any breaking changes (even revving transitive dependencies across major versions), then this package will need a major version rev as well.

<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<PackageId>Microsoft.Azure.DurableTask.AzureServiceFabric</PackageId>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
Expand All @@ -14,29 +15,42 @@
<Authors>Microsoft</Authors>
<Platforms>AnyCPU;x64</Platforms>
<NoWarn>$(NoWarn);NU5104</NoWarn>
</PropertyGroup>

</PropertyGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<Reference Include="System.Web" />
<PackageReference Include="Microsoft.AspNet.WebApi.OwinSelfHost" />
<Compile Remove="Service\net8\**" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net472'">
<Reference Include="System.Web" />
<PackageReference Include="Microsoft.AspNet.WebApi.OwinSelfHost" />
<Compile Remove="Service\net8\**" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<!-- Restoring packages for netstandard2.0 causes warnings. As warnings are treated as errors, compilation will fail. -->
<!-- Once the packages support netstandard2.0, this project will support netstandard2.0. -->
<PackageReference Include="Newtonsoft.Json" />
<Compile Remove="Service\net462\**" />
<Compile Remove="Service\net8\**" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<PackageReference Include="Newtonsoft.Json" />
<PackageReference Include="Microsoft.AspNetCore.Mvc" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" />
<Compile Remove="Service\net462\**" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="ImpromptuInterface" />
<PackageReference Include="Microsoft.AspNet.WebApi.OwinSelfHost" />
<PackageReference Include="Microsoft.AspNet.WebApi.Client" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" />
<PackageReference Include="Microsoft.ServiceFabric" />
<PackageReference Include="Microsoft.ServiceFabric.Data" />
<PackageReference Include="Microsoft.ServiceFabric.Services" />
<PackageReference Include="Microsoft.ServiceFabric" />
<PackageReference Include="Microsoft.ServiceFabric.AspNetCore.Kestrel" />
<PackageReference Include="System.Collections.Immutable" />
</ItemGroup>

Expand All @@ -50,4 +64,5 @@
<PackagePath>content/SBOM</PackagePath>
</Content>
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public Task<long> GeneratePartitionHashCodeAsync(string value, CancellationToken
long hashCode = 0;
if (!string.IsNullOrEmpty(value))
{
using (var sha256 = SHA256Managed.Create())
using (var sha256 = SHA256.Create())
{
var bytes = Encoding.UTF8.GetBytes(value);
var hash = sha256.ComputeHash(bytes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,25 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Formatting;
using System.Net.Sockets;
using System.Threading;
using System.Threading.Tasks;
using System.Web;
using DurableTask.AzureServiceFabric.Exceptions;
using DurableTask.AzureServiceFabric.Models;
using DurableTask.Core;
using DurableTask.Core.Exceptions;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

namespace DurableTask.AzureServiceFabric.Remote
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Formatting;
using System.Net.Sockets;
using System.Threading;
using System.Threading.Tasks;
using System.Web;

using DurableTask.Core;
using DurableTask.Core.Exceptions;
using DurableTask.AzureServiceFabric.Exceptions;
using DurableTask.AzureServiceFabric.Models;

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System.Web.Http.Results;

/// <summary>
/// Allows to interact with a remote IOrchestrationServiceClient
/// </summary>
Expand Down Expand Up @@ -116,16 +113,14 @@ public async Task ForceTerminateTaskOrchestrationAsync(string instanceId, string
instanceId.EnsureValidInstanceId();

var fragment = $"{this.GetOrchestrationFragment(instanceId)}?reason={HttpUtility.UrlEncode(reason)}";
using (var response = await this.ExecuteRequestWithRetriesAsync(
using var response = await this.ExecuteRequestWithRetriesAsync(
instanceId,
async (baseUri) => await this.HttpClient.DeleteAsync(new Uri(baseUri, fragment)),
CancellationToken.None))
CancellationToken.None);
if (!response.IsSuccessStatusCode)
{
if (!response.IsSuccessStatusCode)
{
var message = await response.Content.ReadAsStringAsync();
throw new RemoteServiceException($"Unable to terminate task instance. Error: {response.StatusCode}:{message}", response.StatusCode);
}
var message = await response.Content.ReadAsStringAsync();
throw new RemoteServiceException($"Unable to terminate task instance. Error: {response.StatusCode}:{message}", response.StatusCode);
}
}

Expand Down Expand Up @@ -154,7 +149,12 @@ public async Task<IList<OrchestrationState>> GetOrchestrationStateAsync(string i
{
instanceId.EnsureValidInstanceId();

#if NETFRAMEWORK
var fragment = $"{this.GetOrchestrationFragment(instanceId)}?allExecutions={allExecutions}";
#else
var fragment = $"{this.GetOrchestrationFragmentAll(instanceId)}?allExecutions={allExecutions}";
#endif

var stateString = await this.GetStringResponseAsync(instanceId, fragment, CancellationToken.None);
var states = JsonConvert.DeserializeObject<IList<OrchestrationState>>(stateString);
return states;
Expand Down Expand Up @@ -278,6 +278,8 @@ public async Task<OrchestrationState> WaitForOrchestrationAsync(string instanceI

private string GetOrchestrationFragment(string orchestrationId) => $"orchestrations/{orchestrationId}";

private string GetOrchestrationFragmentAll(string orchestrationId) => $"orchestrationsAll/{orchestrationId}";

private string GetMessageFragment() => "messages";

private string GetMessageFragment(long messageId) => $"messages/{messageId}";
Expand All @@ -286,19 +288,17 @@ public async Task<OrchestrationState> WaitForOrchestrationAsync(string instanceI

private async Task<string> GetStringResponseAsync(string instanceId, string fragment, CancellationToken cancellationToken)
{
using (var response = await this.ExecuteRequestWithRetriesAsync(
using var response = await this.ExecuteRequestWithRetriesAsync(
instanceId,
async (baseUri) => await this.HttpClient.GetAsync(new Uri(baseUri, fragment)),
cancellationToken))
cancellationToken);
string content = await response.Content.ReadAsStringAsync();
if (response.IsSuccessStatusCode)
{
string content = await response.Content.ReadAsStringAsync();
if (response.IsSuccessStatusCode)
{
return content;
}

throw new HttpRequestException($"Request failed with status code '{response.StatusCode}' and content '{content}'");
return content;
}

throw new HttpRequestException($"Request failed with status code '{response.StatusCode}' and content '{content}'");
}

private async Task PutJsonAsync(string instanceId, string fragment, object @object, CancellationToken cancellationToken)
Expand All @@ -308,23 +308,21 @@ private async Task PutJsonAsync(string instanceId, string fragment, object @obje
SerializerSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.All }
};

using (var result = await this.ExecuteRequestWithRetriesAsync(
using var result = await this.ExecuteRequestWithRetriesAsync(
instanceId,
async (baseUri) => await this.HttpClient.PutAsync(new Uri(baseUri, fragment), @object, mediaFormatter),
cancellationToken))
{
cancellationToken);

// TODO: Improve exception handling
if (result.StatusCode == HttpStatusCode.Conflict)
{
throw await (result.Content?.ReadAsAsync<OrchestrationAlreadyExistsException>() ?? Task.FromResult(new OrchestrationAlreadyExistsException()));
}
// TODO: Improve exception handling
if (result.StatusCode == HttpStatusCode.Conflict)
{
throw await (result.Content?.ReadAsAsync<OrchestrationAlreadyExistsException>() ?? Task.FromResult(new OrchestrationAlreadyExistsException()));
}

if (!result.IsSuccessStatusCode)
{
var content = await (result.Content?.ReadAsStringAsync() ?? Task.FromResult<string>(null));
throw new RemoteServiceException($"CreateTaskOrchestrationAsync failed with status code {result.StatusCode}: {content}", result.StatusCode);
}
if (!result.IsSuccessStatusCode)
{
var content = await (result.Content?.ReadAsStringAsync() ?? Task.FromResult<string>(null));
throw new RemoteServiceException($"CreateTaskOrchestrationAsync failed with status code {result.StatusCode}: {content}", result.StatusCode);
}
}

Expand Down
Loading
Loading