Skip to content

Repository is now instanciated on the fly - #440

Open
abienvenu wants to merge 2 commits into
FriendsOfSymfony:masterfrom
abienvenu:onthefly_repo
Open

abienvenu wants to merge 2 commits into
FriendsOfSymfony:masterfrom
abienvenu:onthefly_repo

Conversation

@abienvenu

Copy link
Copy Markdown

No more repository instantiated in constructor of ClientManager and TokenManager.

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

Fixes issue #422

@ruudk ruudk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@ruudk

ruudk commented Jan 5, 2017

Copy link
Copy Markdown

Tests are failing tho

@abienvenu

Copy link
Copy Markdown
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

dinamic commented Jan 31, 2018

Copy link
Copy Markdown
Contributor

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

abienvenu commented Feb 1, 2018 •

Copy link
Copy Markdown
Author

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

@dkarlovi

dkarlovi commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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.

4 participants