Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Adding a MicroKernel that holds service configuration#15820

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
weaverryan wants to merge2 commits intosymfony:2.8fromweaverryan:micro_kernel

Conversation

@weaverryan
Copy link
Member

QA
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRnot yet

Hi guys!

This introduces a new kernel whose goal is to be able to build an application without any external files (and to make loading external files easier, since conditional logic can be in PHP).

This allows you to register services and configure DI extensions right in the kernel. With#15742, this (or a sub-class) would be updated to also allow routes to be configured inside the kernel as well.

There is really only one goal of this PR:

  1. Allow services/extensions to be configured in the kernel.

Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

does this actually need to be exposed ?

@weaverryan
Copy link
MemberAuthor

Agreed on not exposing the Loader - I've just fixed both issues!

@weaverryan
Copy link
MemberAuthor

@stof I went backwards and re-added the getResourceLoader at sha:92e419d. I did this so that I could pass the originalLoaderInterface toconfigureServices(), instead of the wrapperContainerBuilderAwareLoader, which could be more useful in edge cases.

@weaverryanweaverryanforce-pushed themicro_kernel branch 3 times, most recently from2cc23d7 to6278ecaCompareSeptember 18, 2015 13:28
@weaverryan
Copy link
MemberAuthor

I've just drastically simplified this PR and updated the description to describe it. This is ready for review!

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This should maybe be an interface, but the class is so simple and we just use it to fetch the ContainerBuilder andreal Loader out, so I don't know if there's a real-use case to pass other concrete implementation in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I agree with you here

@dupuchba
Copy link
Contributor

@weaverryan can you give me a use case on why you would choose to have a new kernel whose goal is to be able to build an application without any external files ?

Thank you :-)

@weaverryan
Copy link
MemberAuthor

@dupuchba Good question. Here are some reasons:

  1. Remove the idea that Symfony is big and complex. In reality, we're very few steps from being able to create an entire app (services & routes) in a single file. That's a powerful concept to illustrate.

  2. Make Symfony better for creating micro-services and prototyping.

  3. This makes configuration simpler. You can load YAML files like normal, but you also have the option to use logic in here for configuration, which allows you to have environment-specific behavior, without theconfig_ENV.yml setup (which works well, but is overhead if you want to keep things small).

  4. Easier to submit bug reports - instead of cloning the SE, you could build a tiny project with this one file (+ whatever other classes, e.g. form types, needed for the bug).

I think this is superior to the normalKernel. If the SE used this, nothing would change, except that users would be passed theContainerBuilder and have the option to configure it in PHP (this is possible now with the ClosureLoader, but not obvious).

Cheers!

@dupuchba
Copy link
Contributor

@weaverryan you convinced me as at work we are currently migrating to a micro service architecture. Very interesting project!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

what about implementing these methods with an empty body, so that you only need to overwrite the ones you need ?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

good idea - I like that better

@weaverryan
Copy link
MemberAuthor

ping @symfony/deciders

@dunglas
Copy link
Member

👍 another step in the right direction!

@xabbuh
Copy link
Member

Probably I miss an important point here, but I don't see right now how this really makes things much easier.

@weaverryan Can you show a simple example on how I would use this in a real application?

@weaverryan
Copy link
MemberAuthor

@xabbuh I have a (somewhat complex) kernel example here (+route integration, which isn't part of this PR, but you can see how the kernel could handle both):https://gist.github.com/weaverryan/e4b19cabb1d9286c4217

Adding services in php is certainly not objectively better, but I think the approach ofconfigureServicesis better than what we have now - I listed some reasons earlier:#15820 (comment).

Here's the big win for me: End users would now implementconfigureServices instead ofregisterContainerConfiguration and have the option to load an external file (like before), or add services directly (or load external files and then use some php logic to mutate those definitions, e.g. for environment differences).

Make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Usually we put public methods before protected ones. Is there a special reason to keep this method here (for example, to make it easier to spot theconfigureExtensions() andconfigureServices() methods)?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

that was exactly my thinking (making them more visible) - but I'd be happy to change the order if this is a hard rule we want to stick to

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

actually, I just changed the order :)

@xabbuh
Copy link
Member

@weaverryan Thanks for the example. I think I now understand your approach better. :) Besides the minor comments I left, I like this approach. 👍

@weaverryan
Copy link
MemberAuthor

@xabbuh thanks for the review!

@xabbuh
Copy link
Member

👍

Status: Reviewed

@fabpot
Copy link
Member

What about this?

<?phpnamespaceSymfony\Component\HttpKernel;useSymfony\Component\Config\Loader\LoaderInterface;useSymfony\Component\DependencyInjection\ContainerBuilder;useSymfony\Component\DependencyInjection\ContainerInterface;abstractclass MicroKernelextends Kernel{publicfunctionregisterContainerConfiguration(LoaderInterface$loader)    {$loader->load(function (ContainerBuilder$container)use ($loader) {$this->configureExtensions($container,$loader);$this->configureServices($container,$loader);        });    }protectedfunctionconfigureExtensions(ContainerBuilder$c,LoaderInterface$loader)    {    }protectedfunctionconfigureServices(ContainerBuilder$c,LoaderInterface$loader)    {    }}

@fabpot
Copy link
Member

I'm going to submit a PR soon with my own version of a micro-kernel.

@fabpot
Copy link
Member

see#15990

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We should probably add:

$loader->addResource(newFileResource(__FILE__));

@fabpotfabpot mentioned this pull requestSep 29, 2015
@fabpot
Copy link
Member

Closing this one as#15990 provides an easier approach.

@fabpotfabpot closed thisOct 1, 2015
@weaverryanweaverryan deleted the micro_kernel branchOctober 1, 2015 18:18
fabpot added a commit that referenced this pull requestNov 5, 2015
This PR was merged into the 2.8 branch.Discussion----------added a micro kernel| Q             | A| ------------- | ---| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aRelated to#15948 and#15820Commits-------eab0f0a added a micro kernel
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@weaverryan@dupuchba@dunglas@xabbuh@fabpot@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp