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

[5.8] Handle DATABASE_URL env variable for database connection #28096

Closed
wants to merge 1 commit into from

Conversation

mathieutu
Copy link
Contributor

@mathieutu mathieutu commented Apr 3, 2019

Hi,
Many cloud providers use urls env var to configure the database connection (often DATABASE_URL), and many ORM (in many languages) handle them too.

The PR aims to handle those urls.

This is inspired by the Doctrine way to handle them.

This is just a draft, coming directly from my projects, feedback and comments are welcomed.

If someone in the Laravel core team confirms me that this is a wanted feature and that it will be merged, I will be glad to spend more time on it.

Roadmap:

  • Add tests
  • Edit database.php config file to use that class
  • Add a redis part
  • See something else?

Ideas for usage (to challenge, could be easier):

// database.php
[
	'default' => env('DATABASE_URL') ? 'url' : env('DB_CONNECTION', 'pgsql'),
	'connections' => [
		// ....
		'url' => UrlParser::parse(env('DATABASE_URL')),
	],
	// ....
]

Thanks,
Matt'

@mathieutu mathieutu force-pushed the features/database_url branch from c973d57 to c24eebe Compare April 3, 2019 10:24
@GrahamCampbell GrahamCampbell changed the title Handle DATABASE_URL env variable for database connection [5.8] Handle DATABASE_URL env variable for database connection Apr 3, 2019
@taylorotwell
Copy link
Member

Which cloud providers?

@m1guelpf
Copy link
Contributor

m1guelpf commented Apr 3, 2019

Heroku does this, for example

@taylorotwell
Copy link
Member

Please remove all the nullable type-hints. They are useless. Thanks.

@mathieutu
Copy link
Contributor Author

mathieutu commented Apr 4, 2019

@taylorotwell Sorry I don't understand what you mean: do you want that I remove all the type-hint or just the nullable part?
1 - ?string $alias => string $alias
2 - ?string $alias => $alias

Because in the first case, it will not work as I'm actually waiting for a nullable string, and doing checks on the null value.

Do you confirm that you're interested by adding and merging this feature, and so I can invest more time on it? If you need more examples of cloud providers, or ORM, I can look up and link them to you.

@taylorotwell
Copy link
Member

So does Heroku not provide the traditional host name / port? You can only use this specifically formatted URL?

@taylorotwell
Copy link
Member

I'm fine in general with supporting it if Heroku provides no other way to connect to databases.

@mathieutu
Copy link
Contributor Author

This url is pretty standard, and heroku (for example) set this url in your project env variables, and rotate it automatically.

It's also easier to manage in other hosting ways, CI, and local projects because it's only one variable to set/copy instead of several.

I've seen Rails use that one too.

And icing on the cake, database software handle it to directly open your database on a click 😉!
2019-04-06 14 37 32

@taylorotwell
Copy link
Member

taylorotwell commented Apr 8, 2019

Is there somewhere the format of this URL is standardized / documented?

@mathieutu
Copy link
Contributor Author

mathieutu commented Apr 8, 2019

I didn't found anything really agnostically standardized, but each language/framework has its documentation:

At the end, it's the classic url format (same than for http and ftp protocoles or any software)

scheme://username:password@host:port/path?query

so in a database context:

driver://username:password@server:port/database?additional_parameters

Examples:
mysql://localhost:4486/foo?charset=UTF-8
pgsql://user:secret@localhost/mydb
sqlite:///somedb.sqlite

The behavior will be clearer when test will be added.

@taylorotwell
Copy link
Member

Gotcha.

@taylorotwell
Copy link
Member

Feel free to add tests.

@taylorotwell
Copy link
Member

Closing pending inactivity but I do think this is kind of a nice idea if anyone else wants to pick it up.

@mathieutu
Copy link
Contributor Author

Hey folks, I've made another PR : #28308!

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.

3 participants