Build propel base classes and other stuff in the %kernel.cache_dir%

[Seldaek]

It's painful that clearing the cache does not give you a clean slate when working with propel.

[jaugustin]

I don't think it's a good idea to clean base classes on cache:clear

[Seldaek]

Care to say why not? Everything in symfony clears on cache:clear, and yet if you use propel and have some old classes around you'll get weird errors, and you clear the cache and they remain, until you clean your base model directory. What's the advantage of having this not follow the conventions of the framework?

[jaugustin]

I prefer to have base classes near child classes and not in a cache directory.

It take time to build all classes on your model and this only need to be done when your model change.

I think it's better to have a command that clean your base folder on demand.

[Seldaek]

It doesn't take so much time, and it could be rebuilt only when needed (i.e. when the schema or model or whatever is relevant changed), just like everything else works in the framework. Right now you need 20 commands to operate propel, which is a massive pain when you don't know it very well, you're never sure if you should run build:sql first or build:model or model:build etc. There is no reason to make humans remember useless stuff that the computer could very well do on its own.

[havvg]

-1 on the cache:clear - or, if implemented, one should be able to disable that behavior. Imho this alters the behavior of Propel itself, not only the integration into Symfony2 provided by this bundle, especially considering the #185, too.

There are projects required to commit the generated classes, this ability should never be removed- well one could use Propel directly instead of the bundle then. There are PaaS out there, where you actually cannot generate the classes, as there is no Phing available for example.

The only advantage I can see is having different platforms for each environment, like having SQLite memory in your test but using MySQL in prod env. Which seems odd, but may help when you are in need of actual database interaction in your tests - and isn't that trivial on setting it up.

The only command you need to run is propel:model:build, and only if you changed any of your schema files or updated Propel (behaviors) - once. Everything else is optional, being it the integrated migrations, reverse engineering, fixtures, acl, database management.

I don't know of any impact when leaving old classes in place other than the DataWiper (for TableMap classes). Can you elaborate on that topic? If there is an issue, then this should be solved, separately.

On the time consumption part, the model generation can easily double the time required to clear the cache (or warm it). php app/console cache:clear 29.90s user 8.41s system 96% cpu 39.620 total php app/console propel:model:build 28.83s user 5.11s system 97% cpu 34.955 total

[Seldaek]

As far as I know, phing isn't used anymore in propel2?

As for the class, I had something like this happen earlier because I had some old base classes still in the om dir, but the table/model was actually gone in the new code:

$ app/console propel:fixtures:load --yml @FooBundle
Use connection named default in dev environment.

  [Propel] Exception
  Warning: constant(): Couldn't find constant Acme\FooBundle\Model\EventTag::PEER in .../vendor/propel/propel1/runtime/lib/map/TableMap.php

As for model:build being so slow, well maybe that's fixable one way or another.

Anyway I just say this as a propel newcomer, this sucks for usability. And what sucks for usability makes people less likely to keep using it or want to try it. I'm sure when you're familiar with it you know that you're only supposed to run this or that command, and you know when. But from my point of view, if I pull new code and get weird shit, clear the cache and still get weird shit, all I can do except debug propel itself is try running random commands see if they help, and that's a bad experience.

[havvg]

The fixtures part is what I meant, it's not only the DataWiper, but the AbstractDataLoader (my fault there). It breaks if you have the XXXTableMap still in place, but not the om part of the respective model.

The Phing part is true for Propel2, but this bundle currently integrates Propel1.

On the newcomer part, welcome :-) Did you read this http://symfony.com/doc/master/book/propel.html - if so, what issues did you encounter?

[Seldaek]

I didn't read that, I just jumped in an existing propel project and encountered things that I think suck and could be better, so I share it in the hope that you guys can make improvements.

I still don't see any reason why propel stuff should be different from doctrine proxies or any other generated code that ends up in the cache dir. Not following the framework conventions introduce more problems than it solves IMO. I see how you could not agree because you're used to it, but all I ask is that you give it some thought.

[willdurand]

I think it could be nice to put the generated classes into the cache.

Le 31 oct. 2012 à 16:32, Jordi Boggiano notifications@github.com a écrit :

I didn't read that, I just jumped in an existing propel project and encountered things that I think suck and could be better, so I share it in the hope that you guys can make improvements.

I still don't see any reason why propel stuff should be different from doctrine proxies or any other generated code that ends up in the cache dir. Not following the framework conventions introduce more problems than it solves IMO. I see how you could not agree because you're used to it, but all I ask is that you give it some thought.

— Reply to this email directly or view it on GitHub.

[c33s]

i also encountered the problem of having old generated classes which where a pain to manually delete. putting the generated base-classes to cache sounds like a good solution to me.

[Richtermeister]

+1 for putting base classes into the cache dir.

According to the bundle guidelines here the bundle directory should be read-only.

As for automatically generating those classes, maybe that can just be controlled via a config parameter.

[alanbem]

+1

[havvg]

The bundle directory is read-only. If you need to write temporary files, store them under the cache/ or log/ directory of the host application. Tools can generate files in the bundle directory structure, but only if the generated files are going to be part of the repository.

@Richtermeister The guideline also reads that it's valid to generate files, if they are part of the repository, which those classes are. I'm not generating those files into any Bundle namespace at all in my projects.

You cannot even load a model class, if the base class does not exist. This is not what's cache about, imho. You would have to put the generation into the autoloader, if it fails to find a base class. Then you have to go through all schemas again, convert them and find out, whether the requested class is actually one of the generated classes, or just doesn't exist. There you can cache each step's result. This would work, if you start with an empty cache at all. In addition you can add a cache warmer doing this, too.

[Richtermeister]

@havvg I'm not sure base classes should be part of the repository. It seems they should be generated with the propel version that the project uses so you can take of advantage of new features/bugfixes if you choose. Where are you generating your files?

I was thinking the cache warmer would be the right place for model generation. I'm pretty sure that's where Doctrine generates their Proxy classes, which are also required for it to work properly.

[havvg]

@c33s You can run this command, to remove those files from your repository; assuming you got all models under a Model namespace. git rm -rf src/**/Model/**/{map,om} afterwards, you generate the models again, done.

@Richtermeister I got all models within a Acme\Model namespace (+sub namespaces). If Doctrine only works with the cache warmer, that's not a valid solution, as it implies you always run the cache warmer, which is optional (I could clear cache without running them).

[adamdunkley]

+1 on it being handled along side propel's usual cache warmup/clear. Auto-generated things shouldn't be committed to VCS, IMO. We're currently .gitignoring all map/om files, I'm building it in to our build process to have them built – having it done in the way suggested would avoid these roundabouts.

[gharlan]

:+1: for putting base classes into the cache dir and using cache warmup for auto-generation