-
Notifications
You must be signed in to change notification settings - Fork 536
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
Aqueduct Container Services #930
Aqueduct Container Services #930
Conversation
…nto aqueduct-container-services
Do you guys really like the word "service" in this context? I think of something very different from what is described here when I hear "service." |
|
||
You can provide custom Request Handlers to the Container. These request handlers are injected after system handlers but before the component get. Request Handlers allow you to intercept request made to the container and return custom responses. | ||
|
||
An example of this is if I want to have a random color generator. I could create a RequestHandler that when someone makes a request to the Container for `{url:"color"}` will intercept and return a custom `IResponse` of `{ status:200, type:"text/plain", value:"blue"}`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Consider a scenario where you want to create a random color generator. I could create a..." #Resolved
@@ -17,6 +17,8 @@ import { IComponentContext, IComponentRuntime } from "@microsoft/fluid-runtime-d | |||
|
|||
import { ComponentHandle } from "@microsoft/fluid-component-runtime"; | |||
|
|||
import { serviceRoutePathRoot } from "../helpers"; | |||
|
|||
/** | |||
* This is as bare-bones base class that does basic setup and enables for factory on an initialize call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I know this isn't from your change but there's a typo here. Should be "This is a"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely from one of my past prs. lol
// TODO: should this just be "s"? | ||
export const serviceRoutePathRoot = "_services"; | ||
|
||
export interface IContainerService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure these interfaces are necessary, and i think they could create confusion. I'd prefer if container services were just IComponents, and i think these interface could lead people to think container services should be runtime components, which is explicitly an anti-pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also worry that this is limiting. For instance, undo-redo isn't and shouldn't be coupled to aquaduct, but it and other such libraries should be able to participate in the container services.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent some time today playing with different implementations. I landed on this model but with a factory pattern where the factory interface lives in framework-definitions. The reason we want a factory is because I really want to inject the IHostRuntime
object into the service. Having access to this object dramatically extends the quality of these services and it's not possible to do that if you have to create the ContainerService object before you create the ContainerRuntime.
I do agree that a ContainerService should just be an IComponent so I removed the requirement of being an IComponentRouter.
We talked about moving the concept of services to the framework level and if we do that we can't make statements that services will function correctly across frameworks. If we want to enforce that we need to move the logic to the runtime level. If we want loose enforcement we can add it to framework-definitions which I've done in the second iteration. If we want no enforcement (which v1 has and I agree is wrong) we should leave it in the Aqueduct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel this is a lot of complexity for something I'm not sure we need. For the IHostRuntime it's only hard to get if using our simple runtime factory, as it's all hidden, for any complex container they'll likely need to handle that code, and then it's not hard to get the IHostRutime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem still exists even if you're a container writer. The problem is that you need to provide your service to the container on creation. So if you want your service to have a reference to the container (IHostRuntime) you have a circular dependency and need a factory so that you create the service the first time someone asks for it. At this point we have the container.
packages/framework/aqueduct/src/helpers/simpleContainerRuntimeFactory.ts
Show resolved
Hide resolved
|
@anthony-murphy Thank you! I learned something new today. Achievement unlocked. This sentence helped it click for me: "An in-memory service in SOP can be transparently externalized as a web service operation." |
// TODO: should this just be "s"? | ||
export const serviceRoutePathRoot = "_services"; | ||
|
||
export interface IContainerService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent some time today playing with different implementations. I landed on this model but with a factory pattern where the factory interface lives in framework-definitions. The reason we want a factory is because I really want to inject the IHostRuntime
object into the service. Having access to this object dramatically extends the quality of these services and it's not possible to do that if you have to create the ContainerService object before you create the ContainerRuntime.
I do agree that a ContainerService should just be an IComponent so I removed the requirement of being an IComponentRouter.
We talked about moving the concept of services to the framework level and if we do that we can't make statements that services will function correctly across frameworks. If we want to enforce that we need to move the logic to the runtime level. If we want loose enforcement we can add it to framework-definitions which I've done in the second iteration. If we want no enforcement (which v1 has and I agree is wrong) we should leave it in the Aqueduct.
packages/framework/aqueduct/src/helpers/simpleContainerRuntimeFactory.ts
Show resolved
Hide resolved
|
||
The concept of Container Services is simple. We define a specific request route that when queried against returns IComponent objects. The Container Developer can then pass in ContainerServices that other components can query for. These IComponent objects are different from the Components we talked about before in the sense that they do not directly have a `ComponentRuntime` backing them so they can not have Distributed Data Structures. Because they don't contain Distributed State they only exist in memory and will be re-created either with every Container instantiation or on every call (depending on the type). | ||
|
||
Container Services can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finish thought. #Resolved
@@ -17,6 +17,8 @@ import { IComponentContext, IComponentRuntime } from "@microsoft/fluid-runtime-d | |||
|
|||
import { ComponentHandle } from "@microsoft/fluid-component-runtime"; | |||
|
|||
import { serviceRoutePathRoot } from "../helpers"; | |||
|
|||
/** | |||
* This is as bare-bones base class that does basic setup and enables for factory on an initialize call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely from one of my past prs. lol
…nto aqueduct-container-services
In the systems sense service works - since they are behaving like OS level services - vs. meaning a web service. I suppose you could say daemon too if you wanted a split. |
The other tool we have for this is the scope parameter that gets past on load. It's intended to provide host level services. But you could create an aggregate component that merges host level 'services' with those provided by the runtime. I wonder how re-usability will work here. As Scriptor moves to become a reusable component I expect they'll package these up into things the base Scriptor component makes - as opposed to things you register at the runtime level. That way you just drop in your scriptor component and load it up and you're good to go - vs. needing to register a bunch of global services for it to work. I'd expect component developers may also need to inherit some of these services from elsewhere. In the case of Teams/OWA they probably would want you to play with their clipboard/undo if possible - vs. do your own. So you may need to check the host scope regardless and only create your own local versions in the absence of one. The scope passed to the render methods also can give UX specific functionality. The old Component creation could also be aware that the given component doesn't summarize or deal with ops - to be slightly more efficient - in which case code-only components could move to the platform layer as well. The nice part with something like the above is you can deal with handles. In general with handles that can be serialized in a map, etc... we should avoid people having hard coded routes. Should be able to do everything with object references/serialized handles at this point. |
@kurtb you raise a ton go good concerns and I don't see this as the final form of services but a first step. The question of reconciling application scope (services) with container services is something tony and I have talked about a lot and there definitely needs to be a solution there. Currently this doesn't exist but will need to be developed going forward. All the Scriptor comments are well defined. How to construct Containers dynamically is a great question that I don't have a good answer for. We definitely need a way for Services to be re-useable across Containers and I think we have that here (in V2) where they can be packaged and consume independently of the Aqueduct. We do need components to be enhanced by services but we don't have a good idea of what it means for a Component to be dependent on a service. The Scriptor team has been doing a lot of work and thinking a lot about how they can be less dependent on external services. As I said at the beginning. A lot of good questions and problems to solve and I'm not trying to answer them here. I'm just trying to get a V1 concept in that people can start building off of and we can iterate on. |
…nto aqueduct-container-services
@kurt one thing I like about not having these managed by the runtime is there is no attach message. The container code just instantiates what it wants. this solves a bunch of problem around what is created when and by who. I'm also not sure we would want handles to these things, as different hosts may provide different features, so depending where a container is hosted the services could differ on where they come from. I've played around with hosting services at routes that are keyof IComponent, it basically works out that the container have a service registry Map<keyof IComponent, IComponent> . This is cool because you can then check the host scope on all requests to see if the host scope implements the interface, and if does, return that instead of the registered service. |
…nto aqueduct-container-services
…nto aqueduct-container-services
I synced with @anthony-murphy , I've made changes to remove common interfaces and to make the services pattern simpler. Now we will accept a registry of services with an id/creation function. We will then use the factory pattern behind the scenes to ensure that only one is created per container. We still allow developers to access the IHostRuntime object so they can leverage what's available there. These services will only be created once someone asks for them. |
|
||
The Aqueduct framework provides two Container Service Factories as well as an optional base class for building Container Services. | ||
|
||
#### SingletonContainerServiceFactory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update doc please
* @param serviceRegistry - Collection of Container Services | ||
*/ | ||
export const generateContainerServicesRequestHandler = | ||
(serviceRegistry: [string, (runtime: IHostRuntime) => Promise<IComponent>][]): RuntimeRequestHandler => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than an array you the serviceRegistery could be Iterable, which is a bit more flexible, as it supports maps too
|
||
private service: IComponent | undefined; | ||
|
||
public get IContainerServiceFactory() { return this; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed anymore
public get id() { return this.serviceId; } | ||
|
||
public constructor( | ||
private readonly serviceId: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could just make this public, and get ride of the id property
export const generateContainerServicesRequestHandler = | ||
(serviceRegistry: [string, (runtime: IHostRuntime) => Promise<IComponent>][]): RuntimeRequestHandler => { | ||
|
||
const factories: SingletonContainerServiceFactory[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe make this a map for easier lookup later
|
||
const service = await serviceP; | ||
const router = service.IComponentRouter; | ||
if (router) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there is more path, and the service isn't a router i think we should fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
@@ -24,7 +24,7 @@ export class SharedComponentFactory implements IComponentFactory, Partial<IProvi | |||
|
|||
constructor( | |||
private readonly ctor: new (runtime: IComponentRuntime, context: IComponentContext) => SharedComponent, | |||
sharedObjects: readonly ISharedObjectFactory[], | |||
sharedObjects: readonly ISharedObjectFactory[] = [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we should default this on shared component, as you should always be specifying some dds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a fair point now that we have services :)
import { RequestParser, RuntimeRequestHandler } from "@microsoft/fluid-container-runtime"; | ||
|
||
// TODO: should this just be "s"? | ||
export const serviceRoutePathRoot = "_services"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related #977
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments, but looks good.
This is a first run at adding Container Services concept into the Aqueduct.
The goal of Container Services is to service developers who want to write components but don't need those components to have DDSs. This has become a common pattern on Bohemia. Tony wrote the v1 of this in the runtime and I have ported it up to the framework. I believe that the runtime should own the container request handler pattern and the Container Services is just a framework extension of that.
This change doesn't have any components using it (coming soon) but it does add some V1 tests to the aqueduct as well as simple docs in the README that describe Container Services.
There are two types of Container Services provided in this original PR.
The
SingletonContainerService
is probably the most common and will provide a service at a given id with a 1 to 1 mapping with the container. All developers within a container who ask for the object will get the same one back.The
InstanceContainerService
will new up a new object every time a developer asks for one. This could be potentially useful for services that return viewable ui objects where you don't want the views to always be the same. But since I haven't explored maybe this is just throw away. The goal was to provide an extensible interface with two simple implementations.