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.

Sign up for GitHub

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

Jump to bottom

Update MasterSlaveConnection.php #685

Closed
wants to merge 1 commit into from
Closed

Update MasterSlaveConnection.php #685

wants to merge 1 commit into from

Conversation

ChristopheBoucaut
Copy link

If you use close method just before connect method, $this->_conn is unset and we have got an error php.

@ChristopheBoucaut
Update MasterSlaveConnection.php
e5b80ff 
If you use close method just before connect method, $this->_conn is unset and we have got an error php.
@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-997

We use Jira to track the state of pull requests and the versions they got
included in.

@ChristopheBoucaut
Copy link
Author

Sorry, I was wrong. It's just notice, not php error.

@stof
Copy link
Member

stof commented Oct 14, 2014

the right fix would be to fix Connection::close to reset the value of the property instead of unsetting the property from the object

@stof stof mentioned this pull request Oct 14, 2014
Merged
@Ocramius Ocramius self-assigned this Oct 14, 2014
@Ocramius
Copy link
Member

#688 indeed fixes this

@deeky666
Copy link
Member

While I'm fine with the proposed solution by @stof, wouldn't it be more reasonable here to check for $this->isConnected()?

@stof
Copy link
Member

stof commented Oct 21, 2014

I don't understand your proposal

@deeky666
Copy link
Member

@stof nevermind, got the implementation of MasterSlaveConnection wrong. Thought it would set Connection::$_isConnected flag upon connecting but it doesn't. Just forget it :D

Ocramius added a commit that referenced this pull request Oct 22, 2014
@Ocramius
Merge branch 'test/#691-failing-test-for-#685' into hotfix/#685-DBAL-…
27f3ceb 
…997-undefined-_conn-property

Close #691
Ocramius added a commit that referenced this pull request Oct 22, 2014
@Ocramius
Merge branch 'hotfix/#688-hotfix-for-#685' into hotfix/#685-DBAL-997-…
f08d7a2 
…undefined-_conn-property

Close #688
Ocramius added a commit that referenced this pull request Oct 22, 2014
@Ocramius
Merge branch 'hotfix/#685-DBAL-997-undefined-_conn-property'
6f346ad 
Close #685
Close #688
Close #691
Ocramius added a commit that referenced this pull request Oct 22, 2014
@Ocramius
#685 - as per @deeky666's review, exception should be expected only o…
e807227 
…n the quoting call
Ocramius added a commit that referenced this pull request Nov 5, 2014
@Ocramius @deeky666
#685 - as per @deeky666's review, exception should be expected only o…
ee124be 
…n the quoting call
sarcher pushed a commit to sarcher/dbal that referenced this pull request Jan 1, 2015
@Ocramius @sarcher
doctrine#685 - as per @deeky666's review, exception should be expecte…
c9ffb1a 
…d only on the quoting call
This was referenced Dec 6, 2015
Closed
Closed
Closed
@Ocramius Ocramius mentioned this pull request Oct 1, 2016
Merged
1 task
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@Ocramius Ocramius

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ChristopheBoucaut @doctrinebot @stof @Ocramius @deeky666