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

Repository is now instanciated on the fly #440

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

Conversation

abienvenu
Copy link

No more repository instantiated in constructor of ClientManager and TokenManager.

This avoids a database connection to be established for every request.

Fixes issue #422

Copy link

@ruudk ruudk left a comment

Choose a reason for hiding this comment

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

👍

@ruudk
Copy link

ruudk commented Jan 5, 2017

Tests are failing tho

@abienvenu
Copy link
Author

Right, there was a test conflicting with the very purpose of this pull request. I removed this test. There are still some failing tests, but I guess these out of memory errors are unrelated.

@dinamic
Copy link
Contributor

dinamic commented Jan 31, 2018

The unit tests have improved greatly the last couple of weeks.

Could you rebase on top of master?

@abienvenu abienvenu force-pushed the onthefly_repo branch 3 times, most recently from 25b47fa to 4b2db2d Compare January 31, 2018 23:25
This avoids a database connection to be established for every request
Fixes issue FriendsOfSymfony#422
@abienvenu
Copy link
Author

abienvenu commented Feb 1, 2018

Ok @dinamic , I made the rebase and tests are fine now.

@dkarlovi
Copy link
Contributor

dkarlovi commented Feb 1, 2018

Can you handle authcodemanagers too?

This avoids a database connection to be established for every request
Completes fix for issue FriendsOfSymfony#422
@abienvenu
Copy link
Author

Yes @dkarlovi, good point. AuthCodeManagers are handled as well now.

@dkarlovi dkarlovi self-assigned this Feb 14, 2018
@dkarlovi dkarlovi removed their assignment Jun 15, 2023
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.

None yet

4 participants