Add support for authentication using an Azure.Core.TokenCredential #197

sebastianburckhardt · 2022-10-13T16:32:01Z

The following mechanisms are introduced:

in NetheriteOrchestrationServiceSettings, we replaced "ResolvedConnectionString" with a new class "ConnectionInfo". This is a class that contains enough information to construct objects needed for connections (e.g. SDK clients), such as connection strings, OR token credentials and endpoint URI information. We use separate ConnectionInfo members for all types of connections (table storage, blob storage, page blob storage, eventhubs).
Implement ConnectionInfo extension methods that can construct various SDK objects (v12 table clients, v11 storage accounts, EventHubClient, EventProcessorHost) and authenticate HTTP requests
Create a new abstract class 'ConnectionResolver' that resolves the required ConnectionInfo objects before starting the orchestration service. A ConnectionResolver must be run on the settings object before the service can be started. DTFx clients call this explicitly (since they are constructing a settings object in code). DF clients must inject a connection resolver during startup, since they cannot access the settings object directly. This class has several implementations suited for different use cases, and can also be implemented by users directly if desired:
- ConnectionNameToConnectionStringResolver represents the currently implemented way in which names are resolved to connection strings. This is used by default, if the user does not inject a different resolver
- SimpleCredentialResolver has a constructor that takes an Azure.Core.TokenCredential and two names (for storage account and event hubs namespace). It is intended for convenient use by DTFx applications.
- TokenCredentialResolver is an abstract class whose constructor takes an Azure.Core.TokenCredential and that must be overriden to map connection names to resource names (storage account and event hub name). It is intended to be used with DF's current mechanism where connection names are a application-visible concept (e.g. you can specify a particular connection to be used for a DurableClient).
Added two samples that demonstrate how to use the new mechanism for DF and DTFx, respectively.

jviau

It is great to see AAD auth being supported. However, I am a bit concerned with the number of breaking changes here, especially since Netherite is GA. These changes will require a 2.0 release and do we want to do that so soon? Is there a way to accomplish this in a pure additive non-breaking fashion?

jviau · 2022-10-17T16:52:15Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

@@ -114,15 +119,31 @@ NetheriteOrchestrationServiceSettings GetNetheriteOrchestrationServiceSettings(s
                netheriteSettings.HubName = taskHubNameOverride;
            }

+            // connections for Netherite are resolved either via an injected custom resolver, or otherwise by resolving connection names to connection strings
+            var connectionResolver = this.serviceProvider.GetService<DurableTask.Netherite.ConnectionResolver>()


Possible null reference? IServiceProvider is not verified to be non-null.

Yes, I will add that.

jviau · 2022-10-17T16:53:18Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

@@ -48,6 +51,7 @@ public class NetheriteProviderFactory : IDurabilityProviderFactory
        public NetheriteProviderFactory(
            IOptions<DurableTaskOptions> extensionOptions,
            ILoggerFactory loggerFactory,
+            IServiceProvider serviceProvider,


Can we take ConnectionResolver in directly instead of IServiceProvider?

I also recommend to split these changes into a different constructor. This will avoid breaking changes and also if there is an either/or option (provide creds this way or a different way) the two ctors can represent the two different ways.

Yes, it is a runtime breaking change - existing apps that drop in the version of this assembly will encounter MethodMissingException as this is a different ctor signature at runtime.

This constructor is never called by any client code, it is only accessible via DI.
So this is not actually a breaking change.

I was careful to avoid a breaking change for DF users.

I was not careful for DTFx users though, so I will see if I can add some overloads to keep things compatible for them as well.

Many breaking changes can be runtime only. This will break if I take an app compiled against the previous version of DurableTask.Netherite and drop in the new dll. When following semver, you should always be able to drop in a minor version rev without needing to recompile your application.

I have manually tested on the sample function app that I can drop in the new dlls (DurableTask.Netherite.AzureFunctions.dll and DurableTask.Netherite.dll) and the app still runs, without recompilation.

Supporting update without recompilation for any possible situation seems like an unnecessarily high bar.

This is a very common scenario. Consider the dependency graph for all the NuGet packages a given project may bring in - many of these libraries may end up relying on the same library but different versions. Strictly avoiding breaking changes like this allows this to work. If you do introduce breaking changes, these libraries will be incompatible at runtime and cause failures.

Let's say someone has written a companion library for Netherite and has targeted a version before this change. Someone brings both that library and also directly references Netherite with these breaking changes. Depending on what that companion library does, it may be completely non-functional at runtime due to breaking changes.

This constructor is never called by any client code, it is only accessible via DI.

The type and constructor are public, so it falls under the public API and subject to breaking changes constraints. It is possible end users new up this type directly for whatever reason they want.

As for IServiceProvider vs taking in ConnectionResolver directly. This is mixing DI and service-locator patterns. Well, service locator is considered by some to be an anti-pattern. Ultimately, you have the final say with this. But I strongly recommend against using service-locator pattern unless it is absolutely necessary.

I also recommend to split these changes into a different constructor. This will avoid breaking changes and also if there is an either/or option (provide creds this way or a different way) the two ctors can represent the two different ways.

how does DI know which constructor to use if there is more than one?

how does DI know which constructor to use if there is more than one?

I see you have answered this below. There is an attribute for it.

jviau · 2022-10-17T16:58:24Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

-                var service = new NetheriteOrchestrationService(settings, this.loggerFactory);
-
+                var service = new NetheriteOrchestrationService(settings, this.loggerFactory, this.serviceProvider);
+                  


nit: whitespace

jviau · 2022-10-17T16:59:21Z

src/DurableTask.Netherite/DurableTask.Netherite.csproj

@@ -51,6 +51,7 @@

  <ItemGroup>
    <PackageReference Include="Azure.Data.Tables" Version="12.6.1" />
+    <PackageReference Include="Azure.Identity" Version="1.6.0" />


If we are only using TokenCredential abstraction, I think you only need Azure.Core package.

jviau · 2022-10-17T18:13:30Z

src/DurableTask.Netherite.AzureFunctions/IConnectionInfoFactory.cs

+    /// <summary>
+    /// Allows customization of what settings to use for what connection. Can be used to specify TokenCredentials. 
+    /// </summary>
+    public abstract class ConnectionSettingsCustomizer


Can this be removed? I don't see it used anywhere.

Looks like it is dead code left over from some earlier version. Will remove it.

jviau · 2022-10-17T20:45:23Z

src/DurableTask.Netherite.AzureFunctions/TokenCredentialResolver.cs

+                    return ConnectionInfo.FromTokenCredential(this.tokenCredential, eventHubsNamespaceName, recourceType);
+
+                default:
+                    throw new NotImplementedException("unknown resource type");


nit: NotSupportedException seems more fitting.

jviau · 2022-10-17T20:50:26Z

src/DurableTask.Netherite.AzureFunctions/TokenCredentialResolver.cs

+    /// <summary>
+    /// Resolves connections using a token credential and a mapping from connection names to resource names.
+    /// </summary>
+    public abstract class CredentialBasedConnectionNameResolver : DurableTask.Netherite.ConnectionResolver


nit: file name should match type name.

jviau · 2022-10-17T21:03:38Z

src/DurableTask.Netherite/Connections/ConnectionInfo.cs

+                            ResourceName = name,
+                            ConnectionString = null,
+                            TokenCredential = tokenCredential,
+                            HostName = $"{name}.servicebus.windows.net",


if we want to support Netherite in other Azure clouds we will need some follow up work for cloud-specific host names. Or can this be adjusted to take in the full hostname instead of name?

Yes, the hardcoded string here is only meant to be a convenient default.
The connection resolver can set this any other hostname.

jviau · 2022-10-17T21:22:19Z

src/DurableTask.Netherite/OrchestrationService/NetheriteOrchestrationService.cs

        }

        /// <summary>
        /// Creates a new instance of the OrchestrationService with default settings
        /// </summary>
-        public NetheriteOrchestrationService(NetheriteOrchestrationServiceSettings settings, ILoggerFactory loggerFactory)
+        public NetheriteOrchestrationService(NetheriteOrchestrationServiceSettings settings, ILoggerFactory loggerFactory, IServiceProvider serviceProvider = null)


It is preferable to take in the exact dependencies you need (to follow IoC / DI) vs IServiceProvider.

As with the other ctor comment, I recommend avoiding a breaking change by adding a separate ctor which takes in the new params.

I don't prefer using a list of dependencies. It means we have to add/modify constructor overloads every time the list changes which is really inconvenient. Also, this does not work here since not all of the types are public.

How is this a breaking change? the argument is optional.

As mentioned in the other comment, it is breaking because code compiled against the previous dll will fail to find this constructor if the new version is dropped in place.

Ok, I will create an overload then.

jviau · 2022-10-17T21:25:08Z

src/DurableTask.Netherite/OrchestrationService/NetheriteOrchestrationServiceSettings.cs

-        /// The resolved storage connection string. Is never serialized or deserialized.
-        /// </summary>
-        [JsonIgnore]
-        public string ResolvedStorageConnectionString { get; set; }


There are several breaking changes in this file. Are we okay with that? If we follow semver, that would require a major version rev.

The alternative is to try and do additive changes only - retaining old functionality as is and only adding it what you need for the new functionality.

I can see how much work is required to maintain compatibility. Perhaps keeping some deprecated ResolvedConnectionString fields is enough.

Note that this affects only DTFx users, for DF users there are no breaking changes.

I will add a compatibility shim so there are no breaking changes.

Yeah, avoiding breaking changes is very tricky. Here is a list of what is and is not allowed: https://learn.microsoft.com/en-us/dotnet/core/compatibility/

To avoid breaking changes in this PR, I suggest making sure everything is purely additive. Do not remove or change existing method signatures or properties - only add new ones. The implementations then need to make decisions at runtime for which property to use. You can update the getting/setting implementations though. So, an example of what you are trying to update could be like:

Before:

public string OldConnectionString { get; set; }

After:

public ComplexConnectionType NewConnectionWay { get; set; } public string OldConnectionString { get => NewConnectionWay?.ConnectionString; set => NewConnectionWay = new ComplexConnectiontype(value); // also need some logic around going from non-null to null } public class ComplexConnectionType { public ComplexConnectionType(string connectionString) { } // a ctor for the connection string scenario public ComplexConnectionType(TokenCredential credential) { } // a ctor for the new recommended way. Just an example implementation - format this however you need.

We can then remove the shim when we go to 2.0

There are no more breaking changes here.

…quiring recompilation

jviau

Some comments for now. These are all up to you if you want to follow them or not.

jviau · 2022-10-31T18:52:23Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

@@ -48,6 +51,7 @@ public class NetheriteProviderFactory : IDurabilityProviderFactory
        public NetheriteProviderFactory(
            IOptions<DurableTaskOptions> extensionOptions,
            ILoggerFactory loggerFactory,
+            IServiceProvider serviceProvider,


Supporting update without recompilation for any possible situation seems like an unnecessarily high bar.

This is a very common scenario. Consider the dependency graph for all the NuGet packages a given project may bring in - many of these libraries may end up relying on the same library but different versions. Strictly avoiding breaking changes like this allows this to work. If you do introduce breaking changes, these libraries will be incompatible at runtime and cause failures.

Let's say someone has written a companion library for Netherite and has targeted a version before this change. Someone brings both that library and also directly references Netherite with these breaking changes. Depending on what that companion library does, it may be completely non-functional at runtime due to breaking changes.

This constructor is never called by any client code, it is only accessible via DI.

The type and constructor are public, so it falls under the public API and subject to breaking changes constraints. It is possible end users new up this type directly for whatever reason they want.

jviau · 2022-10-31T18:55:52Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

@@ -48,6 +51,7 @@ public class NetheriteProviderFactory : IDurabilityProviderFactory
        public NetheriteProviderFactory(
            IOptions<DurableTaskOptions> extensionOptions,
            ILoggerFactory loggerFactory,
+            IServiceProvider serviceProvider,


As for IServiceProvider vs taking in ConnectionResolver directly. This is mixing DI and service-locator patterns. Well, service locator is considered by some to be an anti-pattern. Ultimately, you have the final say with this. But I strongly recommend against using service-locator pattern unless it is absolutely necessary.

jviau · 2022-10-31T19:06:23Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

@@ -114,15 +118,31 @@ NetheriteOrchestrationServiceSettings GetNetheriteOrchestrationServiceSettings(s
                netheriteSettings.HubName = taskHubNameOverride;
            }

+            // connections for Netherite are resolved either via an injected custom resolver, or otherwise by resolving connection names to connection strings
+            var connectionResolver = this.serviceProvider?.GetService<DurableTask.Netherite.ConnectionResolver>()


I think there is an opportunity to follow DI/IoC with the connection resolver, but still make it ultimately "optional" for customers. The idea would be to move this kind of logic to the DI setup code. I'll have to take a closer look at what existing DI setup code Netherite has, but I think we can definitely get something working.

For example, is INameResolver even used directly here, or is it only used within the ConnectionResolver? If it is only used within the connection resolver, we can do the following:

Add a 2nd constructor which takes in ConnectionResolver and no INameResolver.

The ctor which still has INameResolver just calls that new ctor with the new ConnectionNameToConnectionStringResolver((name) => nameResolver.Resolve(name)) as the connection resolver.

No longer have a INameResolver field. Which is why it is important INameResolver is not used directly anymore. If we still need it directly, will need to rethink this aproach.

Add [ActivatorUtilitiesConstructor] to the new connection resolver ctor (so it is used for DI).

Update the DI setup code to use TryAddSingleton<ConnectionResolver> with a factory which will pull INameResolver from the service container and return new ConnectionNameToConnectionStringResolver((name) => nameResolver.Resolve(name)) as well.

at this point, maybe a constructor for ConnectionNameToConnectionStringResolver which takes in INameResolver may be nice.

What we now have is no observable difference between old and new. However, customers can now inject ConnectionResolver to the DI container and it will be consumed as expected, but we have maintained DI/IoC principals.

However, customers can now inject ConnectionResolver to the DI container

I don't quite understand how they can inject a ConnectionResolver if we already injected our own during our own setup

Empirically, it does seem to work...

The TokenCredentialDF.FunctionStartup calls builder.Services.AddSingleton<DurableTask.Netherite.ConnectionResolver, MyConnectionResolver>();

The DurableTask.Netherite.AzureFunctions.NetheriteProviderStartup calls builder.Services.TryAddSingleton<ConnectionResolver, NameResolverBasedConnectionNameResolver>();

and magically the right things happen. Did I just get lucky or is the DI smart enough to know which one should take precedence?

If we still need it directly, will need to rethink this aproach

I still need it, but that does not really change anything I think. I just leave it there, and go ahead with the rest of the suggestions.

What is happening is builder.Services.TryAddSingleton<ConnectionResolver, NameResolverBasedConnectionNameResolver>(); will first check a service for ConnectionResolver is already registered, only adding NameResolverBasedConnectionNameResolver if no ConnectionResolver is already registered. This gives end users control over the which instance is registered: if they run before you, they can just call AddSingleton. If they run after, they will first need to remove your service registration then add their own (this is possible, IServiceCollection is a fully mutable collection with add/remove semantics).

Apparently it is not necessary for users to make a distinction of what order the code is running in.

I stepped through the sample in the debugger to see why this is working... what I observed is that the user code is running after me and calls AddSingleton, which I would have expected to cause some problem. Instead, everything ends up working as desired. The debugger shows that as a result, immediately after this step, there are two services of this type in the collection:

And when the NetheriteProviderFactory is constructed by DI, it is given the second one of these services, which is the desired result.

It is a bit strange, but perhaps no need to worry since it is working correctly.

jviau · 2022-10-31T19:08:44Z

src/DurableTask.Netherite/Connections/ConnectionInfo.cs

+    using Microsoft.Azure.Storage;
+
+    /// <summary>
+    /// Internal abstraction used for capturing connection information and credentials.


Considering this type is public, should "Internal" be removed from the comment?

jviau · 2022-10-31T19:10:36Z

src/DurableTask.Netherite/Connections/ConnectionInfoExtensions.cs

+    /// <summary>
+    /// Utilities for constructing various SDK objects from a connection information.
+    /// </summary>
+    public static class ConnectionInfoExtensions


Does this need to be public? With all the breaking change discussions in this PR, I recommend only making types public if they are expected to be consumed by end users.

If they need to be public for cross-assembly usage within this repo, we can use internals-visible-to.

These methods are useful if you want to slightly change something (e.g. the host URL) without having to rewrite all of it.

I think they can be internal, yes.

jviau · 2022-10-31T19:23:34Z

src/DurableTask.Netherite/Connections/CredentialShim.cs

+    /// A utility class for constructing a <see cref="Microsoft.Azure.Storage.Auth.TokenCredential"/> 
+    /// from a <see cref="Azure.Core.TokenCredential"/>.
+    /// </summary>
+    public static class CredentialShim


Consider making this internal.

jviau · 2022-10-31T19:25:28Z

src/DurableTask.Netherite/Connections/EventHubsUtil.cs

@@ -24,9 +24,9 @@ public static class EventHubsUtil
        /// <param name="eventHubName">The name of the event hub.</param>
        /// <param name="partitionCount">The number of partitions to create, if the event hub does not already exist.</param>
        /// <returns>true if the event hub was created.</returns>
-        public static async Task<bool> EnsureEventHubExistsAsync(string connectionString, string eventHubName, int partitionCount)
+        public static async Task<bool> EnsureEventHubExistsAsync(ConnectionInfo info, string eventHubName, int partitionCount, CancellationToken cancellationToken)


This is technically a public API contract, but I get this was never meant for public use. Use your judgement here - do you make it separate methods to avoid breaking change or not?

In the next major version rev I definitely recommend evaluating public API surface and making all utilities like this and other implementation details classes internal.

Yep, I was not particularly aware of how ANY public API surface becomes a liability. We can definitely retire some of these in the next major version change.

jviau · 2022-10-31T19:28:23Z

src/DurableTask.Netherite/Connections/SimpleCredentialResolver.cs

+    /// <summary>
+    /// Resolves connections using a token credential, a storage account name, and an eventhubs namespace name.
+    /// </summary>
+    public class SimpleCredentialResolver : ConnectionResolver


A pattern I like to use for base-class -> "simple/default/recommended" implementation, is to make the concrete type a private nested class of the base type, then add a static method ConnectionResolver.FromCredential(...) to get an instance of that class. I like this for two reasons:

Discoverability - users will see they need a ConnectionResolver and then using intellisense can see an easy way to get one right on that abstract type (the FromCredential method).

Public API - we are not committing an entire new type to the public API surface, but rather a single static method. Much more flexible to changes in the futrue.

Yes, that seems like a good way to do it. Will adjust.

Makes sense. Though I prefer to keep the actual classes in their own file. Just made them internal and added public factory methods to the base class.

That approach is completely fine as well. I keep them in their own file as well - but I make the outer class partial and move the nested class to its own file. So, it would be ConnectionResolver.SimpleCredentialResolver.cs.

But not need for that if you already made it internal.

jviau · 2022-10-31T19:30:43Z

src/DurableTask.Netherite/OrchestrationService/NetheriteConfigurationException.cs

+    /// An exception that indicates configuration errors for Netherite.
+    /// </summary>
+    [Serializable()]
+    public class NetheriteConfigurationException : System.Exception


As mentioned, the options pattern does have a validation exception type in it: https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.options.optionsvalidationexception?view=dotnet-plat-ext-6.0

With some refactoring, could leverage the options pattern more directly to avoid having to recreate this ourselves.

I will leave the refactoring of the settings into an options pattern up to a future PR (perhaps for a major version change). It is not a goal of this PR to align Netherite with best DI practices.

jviau · 2022-10-31T19:32:20Z

src/DurableTask.Netherite/OrchestrationService/NetheriteOrchestrationService.cs

+        /// <summary>
+        /// Creates a new instance of the OrchestrationService with default settings
+        /// </summary>
+        public NetheriteOrchestrationService(NetheriteOrchestrationServiceSettings settings, ILoggerFactory loggerFactory, IServiceProvider serviceProvider)


As with the NetheriteProviderFactory, I recommend using DI/IoC over service-locator.

I understand that the service locator may not always be the preferred solution, but I think it is appropriate in this situation.

Not all of the services are public, so it is not just a simple case of listing them in the constructor argument.

Constructors that list all the dependencies create maintenance complexity. In some sense, this is the problem DI is solving in the first place so it seems sad to give up on that. Also, I really dislike having lots of overloads, it makes the code hard to read.

Using a service locator instead helps to keep the constructor stable because new dependencies can be found without having to be listed explicitly.

From what I understand, the danger of the service locator pattern is that the dependency ordering is not visible to the DI startup logic, so it cannot correctly order dependencies during startup. But that is not an issue here because the orchestration service is never constructed at that point. Only the factories are constructed at startup time.

sebastianburckhardt · 2022-11-02T15:26:00Z

There are no more breaking changes, and I have revised some of the DI patterns as recommended. However, I kept the service-locator pattern in one specific situation because I did not like the alternatives. I also added README files to the samples so users understand better what is being demonstrated and how to run them.

jviau · 2022-11-03T17:44:51Z

src/DurableTask.Netherite.AzureFunctions/NetheriteProviderFactory.cs

+        /// <summary>
+        /// This constructor should not be called directly. It is here only to avoid breaking changes.
+        /// </summary>
+        [Obsolete]


Obsolete'ing APIs has some differing viewpoints. .NETs policy is as follows:

An API that shipped in a long-term support (LTS) release must be obsoleted in the subsequent LTS release before it can be removed. In rare cases, exceptions are made to obsolete an API before the subsequent LTS release based on business needs. All obsoletions are documented and communicated to customers.

It takes two LTS releases to remove a public API! First one marks it obsolete, second removes. This is because technically adding [Obsolete] can be viewed as a compilation breaking change if the project has WarnAsError=true.

Buuuut, I think in this case you are fine. This is just more of a fun FYI to express how "fun" maintaining public APIs is 😆

jviau · 2022-11-03T17:48:03Z

src/DurableTask.Netherite/Connections/ConnectionResolver.cs

+        /// Creates a connection resolver for a given connection string lookup function.
+        /// </summary>
+        /// <param name="connectionStringLookup">A function that maps connection names to connection strings.</param>
+        public static ConnectionResolver FromConnectionNameToConnectionStringResolver(Func<string, string> connectionStringLookup)


Is there a more concise name we can use? We can at least drop Resolver suffix, as that is already expressed in the type ("ConnectionResolver"). For a more concise name, maybe FromNamedConnectionString?

I think I prefer the verbose name here. I find it helpful if implementations of an abstract class share the last word with the superclass.

jviau · 2022-11-03T17:49:02Z

src/DurableTask.Netherite/Connections/ConnectionResolver.cs

+        /// Applies the default suffixes (*.core.windows.net for storage accounts and *.servicebus.windows.net for event hubs).
+        /// </summary>
+        /// <param name="tokenCredential">The token credential to use.</param>
+        /// <param name="storageAccountName">The name of the storage account, or null if using in-memory emulation.</param>


Does in-memory require a credential at all? Should an explicit separate method be added instead ForInMemory()?

Yes, I suppose that makes sense.

jviau · 2022-11-03T17:50:24Z

src/DurableTask.Netherite/Connections/ConnectionResolver.cs

+        /// <param name="tokenCredential">The token credential to use.</param>
+        /// <param name="storageAccountName">The name of the storage account, or null if using in-memory emulation.</param>
+        /// <param name="eventHubNamespaceName">The name of the event hub namespace, or null if using the singlehost configuration.</param>   
+        public static ConnectionResolver FromTokenCredentialAndResourceNames(Azure.Core.TokenCredential tokenCredential, string storageAccountName = null, string eventHubNamespaceName = null)


What do you think of dropping "AndResourceNames"? FromTokenCredential(...) sounds fine to me, even including the resource name parameters.

support authentication using token credentials.

7de8a0d

sebastianburckhardt mentioned this pull request Oct 13, 2022

Support identity-based storage connections #149

Open

jviau reviewed Oct 17, 2022

View reviewed changes

sebastianburckhardt added 2 commits October 27, 2022 15:38

address PR feedback

8a5c14f

use overload instead of optional argument to be compatible without re…

e42e75a

…quiring recompilation

sebastianburckhardt added this to the 1.2.0 milestone Oct 28, 2022

sebastianburckhardt linked an issue Oct 28, 2022 that may be closed by this pull request

Support identity-based storage connections #149

Open

jviau reviewed Oct 31, 2022

View reviewed changes

address PR feedback

df48b30

jviau approved these changes Nov 3, 2022

View reviewed changes

address PR feedback

7082941

sebastianburckhardt merged commit ed721c6 into dev Nov 9, 2022

		var service = new NetheriteOrchestrationService(settings, this.loggerFactory);

		var service = new NetheriteOrchestrationService(settings, this.loggerFactory, this.serviceProvider);

Add support for authentication using an Azure.Core.TokenCredential #197

Add support for authentication using an Azure.Core.TokenCredential #197

Conversation

sebastianburckhardt commented Oct 13, 2022

jviau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianburckhardt Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jviau Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jviau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianburckhardt commented Nov 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianburckhardt Oct 27, 2022 •

edited

Loading

jviau Oct 27, 2022 •

edited

Loading