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

MLIBZ-2350 Revert removal of caching db connections in WebSQL. #241

Merged
merged 1 commit into from Feb 16, 2018

Conversation

@georgiwe
Copy link
Contributor

commented Feb 15, 2018

Description

During the mono repo move (I believe. this is the commit here), the caching of database connections was removed. This caused problems when making many operations in websql, since each one would "open" the database. We encountered this in the unit tests suite.

Changes

Add back the caching of "open" databases.

@georgiwe georgiwe self-assigned this Feb 15, 2018

@georgiwe georgiwe requested review from thomasconner and tejasranade Feb 15, 2018

@georgiwe

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

@thomasconner , why was this removed in the first place? Perhaps we shouldn't bring it back? Also, there didn't seem to be a mechanism for closing or removing these from cache. Is that the way it should be?

@tsvetomir-nedyalkov asked me to take this on, since it's causing problems with integration tests and would force him to disable running the tests on phonegap in general. I had tracked it down to this perticular commit, so I figured it won't be too difficult to revert the change. And if this is all that needs doing, we better do it sooner, rather than later, and have a more robust integration test suite.

@georgiwe georgiwe merged commit 77962dc into master Feb 16, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@georgiwe georgiwe deleted the websql-open-database branch Feb 16, 2018

kinson added a commit to cbre360/js-sdk that referenced this pull request Feb 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.