-
Notifications
You must be signed in to change notification settings - Fork 1.5k
DOCSP-36792: Add default database connection admonition #2888
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
DOCSP-36792: Add default database connection admonition #2888
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong, the DB
facade can be used for any connection. Each model class specifies the connection it uses (for Eloquent builder), and the connection name can be specified before creating a query builder:
DB::connection('mongodb')->collection('users')->...
@@ -22,7 +22,7 @@ Configure Your MongoDB Connection | |||
|
|||
.. code-block:: php | |||
|
|||
'default' => env('DB_CONNECTION', 'mongodb'), | |||
'default' => 'mongodb', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If people want to update their default connection, it might be in the env var DB_CONNECTION
in the .env
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should set the default connection to MongoDB even if the DB_CONNECTION
variable is set to something else in .env
. But I updated the admonition (in query-builder.txt) to give two options on how to set MongoDB as the connection, if that helps clarify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remain convinced that this is the not the right way to change the default connection. @alcaeus any opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Laravel's default configuration uses env variables, and I agree that we shouldn't deviate too far from this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I changed these code snippets back to using env variables. Since we don't explain the .env file or its variables in the quick start, I made a ticket to add this info.
@@ -22,7 +22,7 @@ Configure Your MongoDB Connection | |||
|
|||
.. code-block:: php | |||
|
|||
'default' => env('DB_CONNECTION', 'mongodb'), | |||
'default' => 'mongodb', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Laravel's default configuration uses env variables, and I agree that we shouldn't deviate too far from this.
@@ -35,7 +35,7 @@ Configure Your MongoDB Connection | |||
'connections' => [ | |||
'mongodb' => [ | |||
'driver' => 'mongodb', | |||
'dsn' => env('DB_URI', '<connection string>'), | |||
'dsn' => '<connection string>', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the connection string is likely to differ between environments (e.g. dev, staging, prod), we should stick with the Laravel default of providing this in an environment variable.
'dsn' => '<connection string>', | |
'dsn' => env('DB_URI', '<connection string>'), |
docs/query-builder.txt
Outdated
If MongoDB is not your application's default database, you can use the ``DB::connection()`` method | ||
to specify a MongoDB connection. Pass a value of ``"mongodb"`` to the ``connection()`` method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should mention that one should pass the connection name (for which we use "mongodb" in our examples). Also, I suggest using a code snippet for this purpose:
If MongoDB is not your application's default database, you can use the ``DB::connection()`` method | |
to specify a MongoDB connection. Pass a value of ``"mongodb"`` to the ``connection()`` method. | |
If MongoDB is not your application's default database, you can use the ``DB::connection()`` method | |
to specify a MongoDB connection. Pass a the name of the connection to the ``connection()`` method: | |
.. code-block:: php | |
$connection = DB::connection('mongodb'); |
Changes addressed, deferring to Jérôme for final review.
JIRA - https://jira.mongodb.org/browse/DOCSP-36792
Staging -
https://preview-mongodbnorareidy.gatsbyjs.io/laravel/DOCSP-36792/quick-start/configure-mongodb/
https://preview-mongodbnorareidy.gatsbyjs.io/laravel/DOCSP-36792/query-builder/
Checklist