Skip to content
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

@wired_factory refactoring proposal #35

Open
pauleveritt opened this issue Mar 14, 2020 · 17 comments
Open

@wired_factory refactoring proposal #35

pauleveritt opened this issue Mar 14, 2020 · 17 comments

Comments

@pauleveritt
Copy link
Collaborator

From a Jan 10 conversation

tl;dr Rewrite and refactor decorator and injector support to support function DI and clarify usage

Generic Decorator

We want a decorator that can register factory functions as well as dataclasses. Along the way, we want clean up naming and allow overriding how this “creator” happens.

Say you have the following:

def login_factory(container):
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory, LoginService)

We’d like, as the explicit case for a decorated registration:

@wired_factory(for_=LoginService, creator=wired_function_creator)
def login_factory(container):
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)

The changes:

  • We no longer have to write a registry.register_function line
  • The name factory gets expanded to wired_factory to make it explicit
  • It turns the result of the creator callable into a wired factory
  • The creator callable can now handle function-based services instead of just dataclass-based services, through use of specific "creators"

Or, a shorthand notation:

@wired_factory()
def login_factory(container) -> LoginService:
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)

In this case:

  • for_ is deduced from the type annotation for the return type
  • creator is deduced from the wrapped target…if it is a function, we use wired_function_creator

Of course you can use an imperative form. In fact, we already have one: registry.register_factory. We could handle the explicit case above:

def login_factory(container):
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory, LoginService, creator=wired_function_creator)

In existing cases, we don’t need a creator. There’s no prep work for the callable. However, if we change to support DI on functions, we’ll want a creator when registering functions.

Here is a shorthand version of this imperative form:

def login_factory(container) -> LoginService:
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory)

Again, this version:

  • Looks at the argument login_factory and sees it is a function, thus uses creator=wired_function_creator
  • Looks at the type annotation return value of login_factory and uses that type as the second argument

The Calling Pattern

This change makes the two-step dance more explicit.

  • creator is a callable that does some work and returns a callable…yes, it is a factory factory
  • That returned callable is registered

It also makes the two-step dance extensible. I have wanted a “custom injector”. The injector is, in essence, the factory factory. In this case:

@wired_factory(creator=wired_dataclass_creator)
@dataclass
class Greeter:
    settings: Settings
    name: str = 'Mary'

    def __call__(self, customer):
        punctuation = self.settings.punctuation
        return f'Hello {customer} my name is {self.name}{punctuation}'

Thus wired_dataclass_creator is the “injector”. That is, it returns a callable that, when called, will construct the dataclass instance using DI.

If one wanted to customize the injection process, one could make a variation of wired_dataclass_creator. One could automate/hide usage of it by making a custom decorator.

Function DI

Above we discussed:

def login_factory(container):
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory, LoginService, creator=wired_function_creator)

At first glance this seems unnecessary: the wired_function_creator callable would do nothing more than just return login_factory.

But what if we wanted to do DI on functions? For example:

def login_factory(dbsession: Session):
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory, LoginService, creator=wired_function_creator)

In this case, we do need a factory-factory callable registered as the service. wired_function_creator would look at the login_factory signature and see if it wanted just the container. If so, it would immediately return login_factory and mimic current behavior.

Otherwise, it would presume this function wanted DI. It would return a lambda or function that, during service lookup/construction, would be handed the container and do the DI dance.

This could be simplified in the imperative form:

def login_factory(dbsession: Session) -> LoginService:
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory)

…and the decorated form:

@wired_factory()
def login_factory(dbsession: Session) -> LoginService:
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)

Extensible Creators

These creator callables are important. They might also be custom. They are a perfect place to experiment with app-specific or site-specific policies (for example, caching.)

In the case of explicitly naming the creator to use, in the decorator or imperative form, this is already handled. Call the creator you want in that case.

Or is it? Perhaps you want a general case for the creator, but have some context-specific cases as exception. For this, one should treat “creator” as itself a service:

def login_factory(dbsession: Session):
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)
registry.register_factory(login_factory, LoginService, creator=WiredFunctionCreator)

With this, WiredFunctionCreator is a service in the registry which will return the correct creator for this situation. You then have access to wired’s extension/customization/overriding patterns:

  • Register a creator for a particular context
  • Register a creator for a particular name
  • Use ordering (whichever registration happens last) to allow overriding

This would mean wired would need to “scan” for creators before doing a scan for factories. We could make easier by having a basic registry usage, where this was done all in one step, or a more-advanced two-step process of setting up a registry.

Injector Service

With the above, injection is part of the creator step. Albeit a big, hairy part. People might want a custom creator, but not touch the injector. Or vice versa. People might want a different injector for a specific service.

Thus, behind the scenes, creator=WiredFunctionCreator will lookup a WiredFunctionInjector when it wants to do the injection part. The context will the for_ the creator is being used to construct.

Along the way, the current injector will be refactored to make each lookup more granular, to make it easier to write your own injector by re-using pieces of the existing injector. It will also be easier to test (all lookups are currently inlined in a big for loop.)

@mmerickel
Copy link
Owner

I'm a big fan of some form of auto-synthesizing factories, and the dataclass support has been a great start already. My general thinking on this is that it should be a series of synthesizers (factory factories / factory creators) which each get a shot at saying whether they support the supplied object. Probably with the assumption that if the object falls through then it is an error unless it is callable - then it is used directly as a factory.

I would have preferred it be type-based but things like dataclasses and attrs classes cannot be detected by simply an isinstance check. For example attrs.has(obj) or dataclasses.is_dataclass(obj).

I suspect the path forward would be to support a registration mechanism for auto-synthesizers/creators on the ServiceRegistry. Something like registry.register_factory_synthesizer(dataclass_factory_synthesizer). This allows you do then simply call registry.register_factory(some_dataclass, LoginService) and wired will run it through the synthesizer chain to turn it into a factory. It's basically middleware (or a view deriver if you want some precedent in Pyramid) that can wrap the factory.

And of course @factory should be a venusian alias for registry.register_factory.

Putting it all together you'd have something like:

def typed_function_factory_synthesizer(obj):
    if inspect.isfunction(obj) and ...:
        inject = ... # pull the types out of the signature and hang onto them
        def wrapper(container):
            # use container.get(...) to grab necessary services, then invoke obj with them
            return inject(obj, container)
    # return None to let another synthesizer try and wrap it

@factory()
def login_factory(dbsession: Session) -> LoginService:
    dbsession = container.get(name='dbsession')
    return LoginService(dbsession)

There's more middleware-y approaches too if we wanted to cascade / wrap multiple times like passing in (obj, next) into the synthesizer, and having it call next(result). I'm not sure that flexibility is warranted here.

@pauleveritt
Copy link
Collaborator Author

@mmerickel I also thought of middleware/tweens though I didn't think of view deriver.

On your last point, you are likely right, except for one pattern I've already seen: replacing just the injector. In fact, I don't want to replace the entire injector, just its input sources.

@pauleveritt
Copy link
Collaborator Author

@mmerickel When you say:

I suspect the path forward would be to support a registration mechanism for auto-synthesizers/creators on the ServiceRegistry.

How would you imagine that implemented? Just some simple state, like a set(), on the registry and each registration gets added to the set in order, with lookups just running through the synthesizers?

Or is it somehow in the registry, e.g. with subscribers? I doubt that is necessary.

Does the synthesizer function need access to registry, e.g. to perhaps get some config information?

Your if example above needs to end with returning the wrapper right? (I know it's just pseudocode.)

You said elsewhere that for_ and context could be implied. Could you explain the latter?

@pauleveritt
Copy link
Collaborator Author

Is the Extensible Creators section above overkill?

@mmerickel
Copy link
Owner

I think the synthesizer/creator stuff is overcomplicating the API and should probably be avoided at this point. We have two things features we're looking at here:

  1. Support factory-factories (creators) that can take a class or callable and return something that wired expects which is def ServiceFactory(container: Container) -> T. A service factory creator is then just def creator(foo: Any) -> ServiceFactory. You're expected to register a ServiceFactory with wired.

  2. Support decorator-based config using things like venusian.

To me, the interesting part is point 2, with point 1 being something wired doesn't need to care about and worst case scenario just supports a help argument like creator= or injector= or adapter= on the factory. So I think the impetus for the refactoring is just to simply make a version of the factory decorator that is not coupled to the register_dataclass function.

@pauleveritt
Copy link
Collaborator Author

How do you connect a ServiceFactory to a factory? Meaning, if wired doesn't care about injector, where does the existing injector go?

@mmerickel
Copy link
Owner

Right now wired supports one api, which is a callable. There's two approaches here at a high level. Let's assume we're starting with the following class and we want to modify things to fit into wired:

class LoginService:
    ...

Option 1

# more complex decorator api because we don't want to actually modify
# the class object into a function object at module-scope (the decorator
# should return the class object, not a function) so that the class can
# still be imported
@wired_factory(for_=ILoginService, creator=creator)
class LoginService:
    ...

# super easy imperative api, just wrap the thing you want to register in its
# creator to make a factory wired can use
registry.register_factory(creator(LoginService), ILoginService)

Option 2

Do not support creators inside wired. Instead, it could potentially support two types of factories: 1) class object with a __wired_factory__ class method, or 2) a callable.

@wired_factory(for_=ILoginService)
@wired_dataclass
class LoginService:
    ...

# since the class is decorated with wired_dataclass, we can just use it directly here too
registry.register_factory(LoginService, ILoginService)

So in the second case the idea would be that the wired_dataclass would return a version of the object with a special __wired_factory__ class method in it, and wired would pick it up and use it as a protocol.

It's possible to support both options and I'm not really sure why we shouldn't. I lean toward the second one as being the common case. If you were hellbent on less LOC and cleanliness you could merge things into a wired_dataclass api, with optional arguments to do the venusian part.

@mmerickel
Copy link
Owner

To clarify that last point, imagine either @wired_dataclass to simply define the __wired_factory__ on the object which can be used with the imperative api or not versus @wired_dataclass(for_=ILoginService) which would also register the venusian decorator. This feels like something that could be done but maybe confusing and too magical.

@pauleveritt
Copy link
Collaborator Author

Let's focus on Option 2. Are you saying the __wired_factory__ class method is not something the user defined on the class, that it is added dynamically to the class by wired_dataclass? (Apologies for being obtuse.)

Not sure how a class method is better than wrapping.

@mmerickel
Copy link
Owner

The idea is that it could be user defined or the class decorator would add it to the class (or function). No one cares where it came from, but wired would consume it as a standard protocol. Concretely, yes, I was proposing that @wired_dataclass would attach it to the class.

The class method is "better" because it lives with the object and passed around with it instead of having to pass it around separately like in option 1.

@pauleveritt
Copy link
Collaborator Author

Ok, we'll do that.

How would DI for function factories work? We could make it transparent and opt-in (signature with more than container), but you might not want that complexity.

@mmerickel
Copy link
Owner

Functions are objects too. They can also have a __wired_factory__ function attribute on them.

@pauleveritt
Copy link
Collaborator Author

Any thoughts on my second point in "How would..."? Is DI-for-functions ok with you?

@mmerickel
Copy link
Owner

Is the question just whether wired should attempt to do DI directly using the signature? Like for functions that have a typed signature?

@mmerickel
Copy link
Owner

I'm ok with it assuming it is well defined... my first ask would be just to define an injector for functions with tests, and if it's high quality/robust we could have wired use it by default for functions.

@pauleveritt
Copy link
Collaborator Author

@mmerickel How about I start a branch that first has science fiction docs, then a draft injector, and only then a @factory/register_factory implementation?

@mmerickel
Copy link
Owner

Sounds good, doing it in steps definitely helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants