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
Provider and booter overhaul, including class-backed provider sources #202
Conversation
551bd25
to
04be9bf
Compare
Also rename "component" to "provider"
Given providers deal with two containers, this makes the role of this container clearer - it's the target for any eventual registrations from the provider being successfully started
…ntainer off the provider directly
This also puts our two block evaluations as close as possible, so it becomes clearer which is being used where
This method serves double duty - to either store the trigger block, or execute a previously stored trigger block. Having the name "trigger!" makes me think only of the latter, not the former
This gives us consistent before/after callbacks for all steps
I want to make the Lifecycle a DSL only - the Provider is already handling most of the lifecycle management, so it makes sense for statuses to be there too
This seems to be only ever called from the provider itself, plus it doesn't make sense to be in the lifecycle DSL
But ugly tho
080fbfa
to
65456b3
Compare
Thanks for the feedback so far, folks 😄 @solnic, I applied all your suggestions! I'll merge this one tomorrow. Happy for continued feedback post-merge, too, of course! |
def setting(*args, **kwargs, &block) | ||
source_class.setting(*args, **kwargs, &block) |
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.
side note, if we drop 2.6 we can delegate with ...
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'd love to do this. I'm going to make a GH issue for this and will PR it separately: #203
This makes the API documentation properly reflect the implementation behavior
Thanks for the thorough feedback, @flash-gordon! ❤️ |
After renaming "bootable component" to "provider" in #200, this PR completely overhauls the Provider and Booter structure and capabilities.
The biggest change is that it backs the
register_provider
block-based DSL with real classes implementing each provider's behavior, viaDry::System::Provider::Source
. This "Provider Source" is defined by the block-based DSL in two ways:Provider::Source
is created, then the block passed toDry::System::Container.register_provider
is used to define the body of that class. Specifically, it will define real instance methods for each of the lifecycle steps:#prepare
,#start
, and#stop
.Dry::System.register_provider_source
) will have already created and defined a subclass ofProvider::Source
with instance methods for its specific behavior, and then the block passed to.register_provider
is then evaluated in the context of the instance of that source, allowing you to provide config (via the built-inProvider::Source#configure
) or define lifecycle step callbacks (via the built-inProvider::Source#before
and#after
).The block DSL evaluation context has become much simpler in this change, too. There's just two clear contexts, which are hopefully quite clear now that we have class-backed provider sources:
self
is theProvider::Source
subclassprepare
,start
,stop
blocks,self
is theProvider::Source
subclass instanceself
is theProvider::Source
subclass instanceFor the latter two blocks, we no longer need block arguments to access the provider container or the target container. These are available as methods that are already available on
self
in each of the block evaluation contexts (#provider_container
/#container
and#target_container
/#target
). For the initial block, access to the target container is intentionally limited because any taget container access should be done inside the latter two block contexts anyway. This should make the providers easier to reason about because there's now just one way to access the provider/target containers.As the final step of provider setup, a
Dry::System::Provider
is initialized, and is provided its specificProvider::Source
subclass.Provider
takes care of all the standard behavior for each Provider, such as setting up the provider container, which it passes to the Source (among other things) when it initializes it.Provider
also takes care of all the provider lifecycle, making sure that e.g. lifecycle steps aren't run when they don't need to be, and that we merge components from the provider container into the target container after each lifecycle step.With
Provider
taking full responsibility for the standard Provider behavior and lifecycle, this has allowed the (former)Booter
to become much simpler: it's now focused on the loading of the provider files themselves, and then registering, storing, and providing access to the Provider instances. The rest of its behavior is just simple delegations to those Provider instances. I've renamed this toProviderRegistrar
to reflect its new behavior and fit better with the overall set of names we have now.One really nice benefit of the new class-based provider sources is that we can now create our own source classes directly, rather than having to use the DSL only. This can be really helpful for complex providers:
Better yet, class-based provider sources allow us to share state between the lifecycle steps! Previously the only way we could do this was by having to register components in the provider container. For example:
But it doesn't stop there, because this also takes external provider sources to the next level. With class-based sources we can now have our external sources provide their own methods to be used when those sources are used when registering a provider. I've used this approach to straighten out our first-party
settings
provider source, which previously hijacked the providersettings
object (which was intended only for providers to configure their sources) to load settings from env. This always twisted my brain whenever I had to think about it. Now, it's much more straightforward. See the source class:You'll see here that it defines its own
#settings
instance method, which can then be used when the provider source is used to register a provider:This all just works now because of those updated block-based DSL evaluation contexts, with the
register_provider
context above being the instance of the provider source, which lets us provide instance-level settings in a natural way, while still having those settings available to the lifecycle methods of the provider source (because they too are instance methods of the same object), which lets us take objects provided from theregister_provider
block and use it from within the source behavior.I could imagine this being really helpful for complex provider sources. For example, if we wanted to provide one for rom-rb and we wanted to provide lots of targeted hooks for users to supply their own config or behaviour, in our provider source class we could set up as many instance methods as we want for this.
Finally, some other notable changes made in this PR:
settings
and instance-levelconfig
are now powered by dry-configurable rather than us building a customDry::Struct
. This feels much more natural and more in-keeping with our overall ecosystem.group
for our provider sources to:dry_system
(from:system
) in order to set a healthier precedent for third party use (i.e. use your own gem name for your group to ensure no conflicts).These changes should all be fully backwards compatible, with a suite of deprecation shims and warnings in place.