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

Injecting config #869

Open
mattsah opened this issue Jan 24, 2015 · 11 comments
Open

Injecting config #869

mattsah opened this issue Jan 24, 2015 · 11 comments

Comments

@mattsah
Copy link

mattsah commented Jan 24, 2015

It'd be really nice if one could set up propel without having to use symfony's ugly configuration mechanisms. After having searched through the various console command code and code related to configuration, it seemed that although theres an $extraConfig variable, at the earliest levels it's never used and there's no mechanism to get anything in there.

We're forced to create a propel config in a given location, with a given format. Providing a mechanism to bootstrap and provide a ConfigurationManager so we can use our own configuration mechanisms and just inject the PHP array would be great.

@marcj
Copy link
Member

marcj commented Jan 24, 2015

Why not use the php format? propel.php

@mattsah
Copy link
Author

mattsah commented Jan 24, 2015

@marcj the PHP format still requires a separate config within certain constraints. I'm trying to make propel pluggable as the ORM for my framework. It seems odd to have a single config file placed separate, named a specific way, etc... and worse, contains all the information I need for runtime config but can't be used for it without separate parsing.

I'd be fine doing the work to make this happen, but I'd like to know what ohters think the best approach is. From what I can tell every command just creates a new ConfigurationManager independently from the inherited AbstractCommand. I'm not overly familiar with Symfony's console components, but they seem like something of a mess, but perhaps there's a method in there to get a configuration which has been set in a more central location then just fall back to a new instance?

@marcj
Copy link
Member

marcj commented Jan 24, 2015

This is not odd. You can setup the runtime configuration of course also without that config file. Just see what the command config:convert generates. This command basically converts propel.* to plain php code which calls/creates actual php objects necessary to for Propel's runtime. For generator commands it's a bit more tricky, but feasible.

@marcj
Copy link
Member

marcj commented Jan 24, 2015

Or look at the Symfony Propel bundle, which integrates Propel perfectly into Symfony.

@mattsah
Copy link
Author

mattsah commented Jan 24, 2015

@marcj Runtime I'm not so concerned with, because that is easily set up in my startup/bootstrap actions and I can store all that config in my normal configuration structure. It's moreso the generator. If I could do precisely what I could do with the runtime config, but with the generator... that is, configure it at runtime. That'd solve my problem, but this doesn't seem possible without rewriting entire swaths of components... in fact, the commands themselves.

@marcj
Copy link
Member

marcj commented Jan 24, 2015

Or look at the Symfony Propel bundle, which integrates Propel perfectly into Symfony.

It also wraps all commands, so makes it possible to use with Symfony's file structure.

@mattsah
Copy link
Author

mattsah commented Jan 24, 2015

I guess that's what I'll have to do then, is just wrap every command and overload the method that gets the configuration manager. I guess I was trying to avoid a future maintenance headache.

@marcj
Copy link
Member

marcj commented Jan 24, 2015

Or update our AbstractCommand so it fits in your use case, then we can discuss it in a PR so we get it into core.

@mattsah
Copy link
Author

mattsah commented Jan 24, 2015

@marcj I think if I were going to update AbstractCommand I'd just give it a method to set an array config which would be used when creating the new generator config since it already seems to take that as an argument to merge into the loaded config, then rewrite the propel command wrapper to inject that config prior to instantiating and adding the commands. I'm going to play around with the idea of extending or adding a trait to them first as part of my plugin package for the framework though.

@mattsah
Copy link
Author

mattsah commented Jan 24, 2015

@marcj I've gone ahead and just created a package of sorts with a trait for the injector and incorporated it into commands that inherit from the base. This will probably do fine. Thanks for your help / suggestion on the bundle.

@dereuromark
Copy link
Contributor

Is someone able to make a PR here with suggested changes? Or shall we close?

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

No branches or pull requests

3 participants