Combine services into a single service

[tmarkovski]

@sethjback and I spoke briefly at IIW about not using separate services in the SDK, but use one single service. This simplifies a lot of the friction and complexity we have around setting different options, tokens, channels, etc. This only applies to the wrapper service, we can keep protobuf definitions separate. It also seems this change can be introduced seamlessly

Pros:

• Simple SDK usage/importing
• Single configuration
• Easier grpc channel management/reuse
• Simple auth configuration
• We don't have to think about individual service naming
• Can be added without breaking other services
• Will simplify documentation by focusing on the use case, and not the individual service

Cons:

• Extra work, though not too terrible
• Extra work to update samples
• Extra work to update/refactor documentation

Thoughts?

• .NET .NET SDK: Uniservice, New Login, Webhooks (#615, #666, #719) #744
• Go Go 615, 666, 719 #738
• Java Java & Kotlin 615, 666, 719 #739
• Dart Dart work for 615, 666, 719 #736
• Python All the python work for 666, 615, 719 #732
• Ruby Ruby update 615 666 719 #740
• Swift Sdk 615 666 719 sdk-swift#19
• Web Typescript for 615, 666, 719 #733

[fundthmcalculus]

Thoughts

1. Completely agree with the pros. Seems like a good wrapping over the existing services for convenience
2. Would we eventually deprecate the existing service layout?
3. Does this make the .Search() functions somewhat confusing? The benefit of separate services is a separation of concerns.
4. Updating samples and documentation will be quite a bit of work. @MichaelEdwardBlack @fundthmcalculus are already at capacity.
5. I don't think this will actually simplify the documentation, just move things around.
6. If we're going to do it, do it soon, so we can start reworking everything.

[janpieterz]

We would probably need to sort this pre-GA/Beta, I'm guessing?

Updating samples and documentation will be quite a bit of work. @MichaelEdwardBlack @fundthmcalculus are already at capacity.
The whole team could contribute to this, split up samples and docs between all engineers evenly?

[geel9]

It depends on the proposed topology.

Are we talking about nesting instances of each Service inside a single Trinsic instance?

IE, trinsic.Wallet.Search()

My concern with not nesting is that I think we'll end up re-namespacing anyways.

For example -- it seems likely we'd end up with say, SearchWallet() and SearchTrustRegistries() methods. We're still namespacing them, just differently.

And what would trinsic.InsertItem() mean? Insert into what? Would we leave that name as is, or change it to something like InsertItemIntoWallet()?

Overall I'm a +1 on this. Instantiating tons of different Services gets pretty messy. Definitely prefer a "want to use Trinsic? Make a Trinsic" approach.

[fundthmcalculus]

We would probably need to sort this pre-GA/Beta, I'm guessing?

Updating samples and documentation will be quite a bit of work. @MichaelEdwardBlack @fundthmcalculus are already at capacity.
The whole team could contribute to this, split up samples and docs between all engineers evenly?

Definitely. If we're going to do it, it needs to be done soon. It also needs to be a whole-team effort to get everything up to date.

[sethjback]

Are we talking about nesting instances of each Service inside a single Trinsic instance?

Yes, at least that is what @tmarkovski and I discussed.

I think we could do this without too much trouble, and without really breaking anything. Having services namespaced is convenient, but completely isolating them seems to adds friction. We might even be able to get away with just extending and making public the base service, but I think it would also be possible to just create another trinsic service and leave everything else alone.

I envision it working by creating a trinsic service that just wraps the others. You could use the other directly just like now, or you could create a trinsic and get all the other services from there (e.g. trinsic.Wallet().Search()). From what I can tell this shouldn't change any of the existing functionality?

Would we eventually deprecate the existing service layout?

If we are just implementing a wrapper I don't think we would need to, though it might be cleaner.

[fundthmcalculus]

If we are just implementing a wrapper I don't think we would need to, though it might be cleaner.

I'd rather keep things the way they are, and layer on the channel management for the user. In general, I don't think people are trying to handle channel management themselves. I think just handling this automatically would be ideal from a customer perspective.

Keep the existing services where they are, instantiate the base service (or something else), and it handles the account profile management and channel management for you?

[tmarkovski]

I'd rather keep things the way they are, and layer on the channel management for the user. In general, I don't think people are trying to handle channel management themselves. I think just handling this automatically would be ideal from a customer perspective.

Keep the existing services where they are, instantiate the base service (or something else), and it handles the account profile management and channel management for you?

Base service approach doesn't solve the problem, as it still requires separate instances.

Keeping everything in a single service allows us to add central management for the entire application. When a user is authenticated, we can set that globally for them, instead asking them so set it for every service in their DI collection.
Additionally, we can rely on all services using a single grpc channel, which isn't possible if services create a channel for themselves, and wanting to do channel management, requires the user to pass it along. If this is in a central service instance, we can ensure proper configuration.
Also, we can do things like retry resilience, add custom interceptors, etc. There's really no good reason for us to structure everything in separate services, when we have dependency on correct configuration and management for each of them.

[fundthmcalculus]

That's true. How would we want to handle base service instances wherein you have different AccountProfile that you are using? We could still handle channel management for sure, but do they have to provide an override AccountProfile? Do they just set it as we currently do?

The more I think about this, the more I think it's the right thing to do, but needs to be done right, and done now. If this delays the GA release, so be it.

[geel9]

It'd be nice if we could have a WithAuthToken() method on the base service to allow for one-off API calls with a separate auth token

For example, if you wanted to make a single call with a different auth token, but then switch back after, you'd have to do something like:

trinsic.SetAuthToken(newToken);

trinsic.Wallet().InsertItem(...);

trinsic.SetAuthToken(originalToken);

Whereas with the propose method, we could do:

// Does not modify auth token of `trinsic`
trinsic.WithAuthToken(newToken).Wallet().InsertItem(...);

It might take some cleverness (or hackery) to do this in a performant way where we aren't making tons of copies of the base services. The naive implementation of WithAuthToken would just make a clone of the base service object (probably handling grpc channel stuff so a new channel isn't made), set the clone's auth token, and return it.

Alternatively, if the sub-service accessors will always be getter methods (eg trinsic.Wallet()), we could add an optional auth token parameter to these accessors; say, trinsic.Wallet(authToken|null).InsertItem() which would maybe do something similar.

[tmarkovski]

I think we can all agree that developers won't be switching tokens a lot. This is a scenario we run into because we write tests and demos that showcase multiples personas. This is not a common use case otherwise.
In the event that developers do want to do token management, they can set the AuthToken in the service options before making a call.
The difference is doing it on one service is a lot easier, especially if our SDK instantiates that service as singleton, which shouldn't be enforced. However, if language specific practices allow this, we should encourage it.

[sethjback]

Some of the credential concerns might also be alleviated if we move to more of a "context" type system for storing user profiles. See #493 (comment) .

This will also complicate things and for now really isn't necessary. Just throwing it out there FWIW.

[fundthmcalculus]

Some of the credential concerns might also be alleviated if we move to more of a "context" type system for storing user profiles. See #493 (comment) .

This will also complicate things and for now really isn't necessary. Just throwing it out there FWIW.

I know we're going to need this kind of complexity, so I'd rather support it sooner rather than later.

[tmarkovski]

Let's summarize this:

• We want to add single service so that developers need to import only one reference
• Construction of this service/import should remain similar, i.e. accepts a SdkConfig/Options object, with default values available
• The service contains navigation property/method, where applicable, to group domain specific services, for example trinsicService.Templates.Search() or trinsic.templates().search(), whichever makes sense. Is this possible idiomatically on all languages?
• Or, keep a single service, and add the domain specifier to the method name, e.g. trinsic.SearchTemplates(), trinsic.SearchWallet, trinsic.InsertItemWallet(), etc.
• Add this service experimentally, without deprecating the others. Reuse existing proto service definitions. Ensure single channel within the service scope is used for all grpc clients.

Anything else?

[fundthmcalculus]

We want to add single service so that developers need to import only one reference

I like this, it also reduces the chances of our classes poisoning theirs and requiring manual namespace resolution.

Construction of this service/import should remain similar, i.e. accepts a SdkConfig/Options object, with default values available

Definitely. The current ServiceBase() constructor works well for everything.

The service contains navigation property/method, where applicable, to group domain specific services, for example trinsicService.Templates.Search() or trinsic.templates().search(), whichever makes sense. Is this possible idiomatically on all languages?

With the possible exception of go (@sethjback what say you?), yes it is. We can keep the existing protobuf services, and just expose the existing service classes as properties on the TrinsicKitchenSinkService.

• Or, keep a single service, and add the domain specifier to the method name, e.g. trinsic.SearchTemplates(), trinsic.SearchWallet, trinsic.InsertItemWallet(), etc.

👎 I dislike using prefixes to notate logical namespaces. We have the language constructs to properly scope, let's use them. Se my comment above.

• Add this service experimentally, without deprecating the others. Reuse existing proto service definitions. Ensure single channel within the service scope is used for all grpc clients.

Yes. This is the way. We provide a clear upgrade path without breaking our customers existing code. Eventually, we might change, but we can decide when to do that.

Anything else?

Let's get this done sooner rather than later. I say target 1.6 for this set of changes.

[fundthmcalculus]

Is this still a discussion point and/or something we plan to do?

[geel9]

I'm personally a very big +1 on this. Call me a +2. Never before seen.

The amount it would streamline our injected code samples alone makes it well worth it to me.

Big win for clients, big win for us.

[fundthmcalculus]

I'm personally a very big +1 on this. Call me a +2. Never before seen.

The amount it would streamline our injected code samples alone makes it well worth it to me.

Big win for clients, big win for us.

Let's do it. :D