8000 Adding the database to the DSN we are sending to the MongoDB server by Wotre · Pull Request #4640 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Adding the database to the DSN we are sending to the MongoDB server #4640

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

Merged
merged 1 commit into from
Jun 26, 2012
Merged

Adding the database to the DSN we are sending to the MongoDB server #4640

merged 1 commit into from
Jun 26, 2012

Conversation

Wotre
Copy link
Contributor
@Wotre Wotre commented Jun 23, 2012

Adding the database to the DSN we are sending to the MongoDB server.

According to the documentation from PHP the database will default to admin if it isn't specified in this DSN. Unfortunately the username we're trying to login with shouldn't have access to this database.

@travisbot
Copy link

This pull request passes (merged 2251be90 into 0d4b02e).

@@ -114,7 +114,7 @@ protected function getMongo()
{
if ($this->mongo === null) {
if (preg_match('#^(mongodb://.*)/(.*)/(.*)$#', $this->dsn, $matches)) {
$mongo = new \Mongo($matches[1]);
$mongo = new \Mongo($matches[1] . '/' . $matches[2]);
Copy link
Contributor

Choose a reason for hiding this comment

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

You should check if it's not empty before using it, as an empty value will fail now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the valid comment, fixed in 45d0748 :)

@travisbot
Copy link

This pull request fails (merged 45d0748b into 0d4b02e).

@Wotre
Copy link
Contributor Author
Wotre commented Jun 25, 2012

It looks to me like travisbot failed because of an error in the routing system that was fixed in c67cf8b, not because of the code I altered.

@@ -114,7 +114,7 @@ protected function getMongo()
{
if ($this->mongo === null) {
if (preg_match('#^(mongodb://.*)/(.*)/(.*)$#', $this->dsn, $matches)) {
$mongo = new \Mongo($matches[1]);
$mongo = new \Mongo($matches[1] . ($matches[2] != null && !empty($matches[2]) ? '/' . $matches[2] : ''));
Copy link
Member

Choose a reason for hiding this comment

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

should be !empty($matches[2])

@travisbot
Copy link

This pull request passes (merged aa659463 into 0d4b02e).

@fabpot
Copy link
Member
fabpot commented Jun 26, 2012

Can you squash your commits before I merge? Thanks.

@Wotre
Copy link
Contributor Author
Wotre commented Jun 26, 2012

I think I've managed to do that, but correct me if I've done something wrong :)

@travisbot
Copy link

This pull request passes (merged dcb79089 into 0d4b02e).

@fabpot
Copy link
Member
fabpot commented Jun 26, 2012

@Wotre Unfortunately, that's wrong. You can read how to do that in the contrib docs: http://symfony.com/doc/current/contributing/code/patches.html#rework-your-patch

@Wotre
Copy link
Contributor Author
Wotre commented Jun 26, 2012

Thanks for the help, looks like I forgot the -f when pushing. It should be okay now

fabpot added a commit that referenced this pull request Jun 26, 2012
Commits
-------

81d0552 Adding the database to the DSN we are sending to the MongoDB server

Discussion
----------

Adding the database to the DSN we are sending to the MongoDB server

Adding the database to the DSN we are sending to the MongoDB server.

According to the [documentation from PHP](http://be2.php.net/manual/en/mongo.construct.php) the database will default to admin if it isn't specified in this DSN. Unfortunately the username we're trying to login with shouldn't have access to this database.

---------------------------------------------------------------------------

by travisbot at 2012-06-23T13:54:28Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1688817) (merged 2251be90 into 0d4b02e).

---------------------------------------------------------------------------

by travisbot at 2012-06-25T11:34:17Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1700214) (merged 45d0748b into 0d4b02e).

---------------------------------------------------------------------------

by Wotre at 2012-06-25T12:16:49Z

It looks to me like travisbot failed because of an error in the routing system that was fixed in c67cf8b, not because of the code I altered.

---------------------------------------------------------------------------

by travisbot at 2012-06-25T16:45:12Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1702410) (merged aa659463 into 0d4b02e).

---------------------------------------------------------------------------

by fabpot at 2012-06-26T05:07:37Z

Can you squash your commits before I merge? Thanks.

---------------------------------------------------------------------------

by Wotre at 2012-06-26T12:02:02Z

I think I've managed to do that, but correct me if I've done something wrong :)

---------------------------------------------------------------------------

by travisbot at 2012-06-26T12:05:19Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1710220) (merged dcb79089 into 0d4b02e).

---------------------------------------------------------------------------

by fabpot at 2012-06-26T12:14:28Z

@Wotre Unfortunately, that's wrong. You can read how to do that in the contrib docs: http://symfony.com/doc/current/contributing/code/patches.html#rework-your-patch

---------------------------------------------------------------------------

by Wotre at 2012-06-26T12:37:59Z

Thanks for the help, looks like I forgot the -f when pushing. It should be okay now
@fabpot fabpot merged commit 81d0552 into symfony:master Jun 26, 2012
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
0