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

describe cache shouldn’t create all connections #49

Open
wants to merge 2 commits into
base: 1.0
Choose a base branch
from

Conversation

NaderCHASER
Copy link

Resolves #48

@mariuswilms
Copy link
Member

mariuswilms commented Jan 29, 2017

Thanks for providing a patch for this issue. As this tends to be more like a bugfix, can you please submit against the 1.0 branch and use long array syntax. 1.0 was still PHP <5.4.

http://li3.me/development#bugs

* Stays consistent with long array syntax used elsewhere in file / branch.
@NaderCHASER NaderCHASER changed the base branch from master to 1.0 January 30, 2017 17:16
@mariuswilms
Copy link
Member

Just had a deeper look into the issue. Wouldn't disabling autoCreate here, lead to none of the connections having schema caching enabled?

@NaderCHASER
Copy link
Author

You're right. The alternative (previous) approach initiated every connection on every action, though.

I'm not sure how to tackle this yet. The problem lies within the fact that autoConnect defaults to true, leading Connections::get($name) to initiate the connection, regardless of whether or not it's being used for that action. There has to be a way to set autoConnect to false through Connections::get(). I thought it was [ 'config' => [ 'autoConnect' => false ] ] but that didn't appear to work.

@mariuswilms
Copy link
Member

Sorry for not getting back earlier. Yes, there's no obvious bug fix here, that can be applied to just this repo's code. Looks like we need to dig deeper and probably patch Connections.

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

Successfully merging this pull request may close these issues.

2 participants