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

Support for database URLs #729

Merged
merged 16 commits into from
Dec 4, 2014
Merged

Support for database URLs #729

merged 16 commits into from
Dec 4, 2014

Conversation

dzuelke
Copy link
Contributor

@dzuelke dzuelke commented Nov 26, 2014

With a bunch of tests.

Of note:

  1. in the case of information present in both URL and "normal" parameters, I'm currently giving priority to the information from the URL; IMO this makes more sense than vice versa (base info would be defined in params, and each developer or environment has a URL that may or may not override that base info, such as charset)
  2. the syntax for SQLite is sqlite:///relativepath.db or sqlite://ignoredhost/relativepath.db for relative, and sqlite:////tmp/absolutepath.db or sqlite://ignoredhost//tmp/absolutepath.db for absolute paths to the database file (I went back and forth on this, but this way is easier and more consistent; https://github.com/kennethreitz/dj-database-url does the same)
  3. extra query params are simply "copied" into $params verbatim; makes sense IMO especially considering point number 1
  4. the URL/driver mappings may need some review

@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-1050

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

* URL extracted into indidivual parameter parts.
*
*/
private static function _parseDatabaseUrl(array $params)
Copy link
Member

Choose a reason for hiding this comment

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

doesn't need to be prefixed with _ anymore (was an old convention)

@Ocramius
Copy link
Member

Also required for this PR: documentation changes in configuration.rst

@beberlei
Copy link
Member

beberlei commented Dec 1, 2014

@dzuelke Can you add a paragraph to docs/en/reference/configuration.rst? Then its good to merge.

@kingcrunch
Copy link

The four slashes look quite strange in my eyes.

What I want to say: That looks unusual and unintuitive to me. Why don't you simply use DSN?

sqlite::memory:
sqlite:/foo/bar.db
mysql:host=localhost;port=3307;dbname=testdb

Anyway: Like to see this merged (in any form), because I find it annoying to have all this parameters for a single connection 😄

@beberlei
Copy link
Member

beberlei commented Dec 4, 2014

@kingcrunch This is modelled after URIs that IaaS services provide on various platforms, i.e. Amazon RDS, Heroku and so on. This allows developers to use what is provided without further parsing.

@kingcrunch
Copy link

Now I could ask, why they used this format instead of DSNs, but that would go to far 😉 I'm convinced (not, that it matters somehow)

@dzuelke
Copy link
Contributor Author

dzuelke commented Dec 4, 2014

There's your docs, @beberlei and @Ocramius :)

beberlei added a commit that referenced this pull request Dec 4, 2014
@beberlei beberlei merged commit 7d17c01 into doctrine:master Dec 4, 2014
@beberlei
Copy link
Member

beberlei commented Dec 4, 2014

@dzuelke thanks,merged

@dzuelke
Copy link
Contributor Author

dzuelke commented Dec 4, 2014

🚀

@deeky666 deeky666 added this to the 2.5 milestone Jan 5, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants