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

Unable to use php74 preloading because of how table maps are dumped #1695

Closed
prgTW opened this issue Jan 11, 2021 · 13 comments
Closed

Unable to use php74 preloading because of how table maps are dumped #1695

prgTW opened this issue Jan 11, 2021 · 13 comments
Labels

Comments

@prgTW
Copy link
Contributor

prgTW commented Jan 11, 2021

image

@prgTW
Copy link
Contributor Author

prgTW commented May 25, 2021

references #361

@mringler
Copy link
Contributor

I have created a PR (#1742) which should fix this issue. If you have time to test it, I'd be happy to get some feedback

@emicks
Copy link

emicks commented Jun 8, 2021

I am using propelorm/Propel2 together with SkyFoxvn/PropelBundle in a Symfony 4.4 application. PR #1742 breaks the application even if the model has been rebuilt, because the database connections are configured via PropelBundle (config coming from config/packages/propel.yaml in the symfony application) and the generated loader script is not included. Is there any workaround for this?

@mringler
Copy link
Contributor

mringler commented Jun 8, 2021

Interesting, thank your for reporting.
What error message are you seeing?

@emicks
Copy link

emicks commented Jun 8, 2021

I'm getting a "Database map was not initialized. Please check the database loader script included by your conf" error. After (manually) including the generated loadDatabase.php in PropelBundle::configureConnections(), it seems to work. As far as I understood PR #1742, the generated loader is included at the end of the (PHP) config script. However, with PropelBundle, the config is loaded from YAML and connections are initialized in PropelBundle::boot().

@mringler
Copy link
Contributor

mringler commented Jun 8, 2021

I am not familiar with PropelBundle, so it is easy for me to say that the problem lies there. My guess is that it rebuilds the config (config.php) from your propel.yaml, otherwise you would see a different error. But then the generated config points to a dummy loader script instead of the one you generated during model:build.
If you remove the manually added include, and instead set the paths.loaderScriptDir property in your propel.yaml, does it work?
Should look something like:

propel:
  database:
      ...
  paths:
      loaderScriptDir: /path/to/dir/of/loader/script

@mringler
Copy link
Contributor

mringler commented Jun 8, 2021

Alright, this is just a guess, but it seems like PropelBundle is not generating a conf file at all, but rather sets the properties manually (in PropelBundle.php). And it looks like it uses only specific paths parameter, so paths.loaderScriptDir will not do anything.
If that is indeed the case, there isn't much we can do from here. It needs to be fixed in PropelBundle.

@emicks
Copy link

emicks commented Jun 8, 2021

AFAIK, PropelBundle does not use a conf.php. With PropelBundle, configuration is loaded from a .yaml file according to the bundle's config info which does not recognize loaderScriptDir : https://github.com/SkyFoxvn/PropelBundle/blob/5.0/DependencyInjection/Configuration.php
The connections are then initialized in the boot code of the bundle. If I require_once the generated loader in PropelBundle::configureConnections() (line 59 in https://github.com/SkyFoxvn/PropelBundle/blob/5.0/PropelBundle.php ) it works.
Perhaps your initial assumption that "the configuration is the only file that is imported directly into a Propel project" could be true only for standalone projects, but not for Symfony projects with PropelBundle.

@emicks
Copy link

emicks commented Jun 8, 2021

I have opened a new issue with SkyFoxvn/PropelBundle so it can be fixed there.

@mringler
Copy link
Contributor

mringler commented Jun 8, 2021

Alright, that sucks. Seems like they implemented their own way of configuring Propel, and now the two packages are tightly coupled.
Easiest fix I can think of is for them to check required config version, and if necessary have their model:build command (I think they ship their own, right?) set a location for the loader script and then, if necessary, load it during bootstrapping.

A way to check if manual loading is required would be something like

if(defined('\\Propel\\Runtime\\ServiceContainer\\StandardServiceContainer::CONFIGURATION_VERSION') 
  && StandardServiceContainer::CONFIGURATION_VERSION >= 2) {
    ...
}

@SkyFoxvn
Copy link

SkyFoxvn commented Jul 3, 2021

@mringler yes the bundle configures Propel by his own way. The idea of the bundle is to be simple a wrapper for Propel which basically mean to save you time for configurations by preconfiguring the propel more specifically to work with Symfony file architecture

@dereuromark
Copy link
Contributor

We can close this then as there is nothing we can do here on our side as follow up?

@mringler
Copy link
Contributor

I think @SkyFoxvn implemented this in the wrapper, nothing to do on Propel side. So this can be closed.

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

No branches or pull requests

5 participants