Fixed #28236 - Integrate dj-database-url into Django #16331

rtpg · 2022-11-27T07:11:11Z

Ticket: https://code.djangoproject.com/ticket/28236

(EDIT: started discussion in the mailing list)

This is a revival of #8562, and I've gone in and rebased, along with adding some documentation. Having some Restructured Text issues regarding function definitions, so I'm clearly doing something wrong with the documentation, I would appreciate pointers on there.

The core thing here is I tried to get this to a state to where we can ship configure_db as a helper to ingest Database URLs. I think this is very important for us to integrate given how ubiquitous database URLs have become. the cache-related URLs are a bit more confusing for me, though.

While I think it makes sense for people to use this as a default, given this is a new thing, I think it's not a good idea to say "OK, now the default template uses this" just yet. There are likely hard edges to this that need to be improved with contact with usecases beyond Heroku/fly.io/whatever.

EDIT: Just to clarify what this looks like, an initial document snippet:

Database URL Support
--------------------

Database connection URLs can be used to configure database backends by calling
:func:`django.conf.service_url.configure_db`::

    import os
    from django.conf.service_url import configure_db

    DATABASES = {
       "default": configure_db(os.environ['DATABASE_URL'])
    }

pauloxnet · 2022-11-27T08:31:19Z

Thanks for working on this . From a first review to your PR I saw you are integrating also cache url , can you create a separated PR for that?

pauloxnet · 2022-11-27T08:32:19Z

django/conf/service_url.py

+
+register_db_backend("mysql", "django.db.backends.mysql")
+register_db_backend("oracle", "django.db.backends.oracle")
+register_db_backend("postgres", "django.db.backends.postgresql")


I think we need to support also additional term here (i.e. "postgresql")

carltongibson · 2022-11-27T09:54:05Z

This is marked Someday/Maybe on the ticket, pending a consensus to bring it into core.

Is there a Django-developers discussion? The package is currently living happily in Jazzband, and has been for quite a while now (I think) Is there a reason why it needs to be merged? (Was that considered on the mailing list?)

rtpg · 2022-11-27T10:46:51Z

@carltongibson https://groups.google.com/g/django-developers/c/G0TtlhH2RUE/m/ShwTToPDAgAJ

This thread was the original discussion that lead to the PR. I posted a reply in that thread earlier, but the message might be in some moderation queue? Could have sworn I was in that mailing list before...

Rereading it more closely now and the ticket, I am now less likely to be sure there's a consensus. Should I start a new discussion up in there?

The short version of the impetus is that DATABASE_URL is an extremely common convention, and making it possible to use Django on PaaSes without other libraries feels like an important goal, along with (most) of this code being straightforward.

PaaS users are often presented with credentials only in this format. This isn't merely a "it's nice to have one env var instead of N env vars" thing.

But yeah, if this feels like we don't have a concensus I'll start a new discussion in the mailing list to get it somehow.

As to the cache code, it was in the original commit but I'm very comfortable removing that. My objective here is to make DATABASE_URL be pretty painless, the rest is just bonus points in my book.

carltongibson · 2022-11-27T11:37:53Z

Please see also: https://groups.google.com/g/django-developers/c/UQjpzB39JN0/m/XGqdV8nbBwAJ — where it was discussed moving the project under the Django org, prior to the Jazzband move.

Note JKM's comment:

I suspect this is a "good enough is good enough" situation. Something like what
Raffaele is talking about, or dsnparse, or whatever would probably be ideal. And
for something to be merged into core, I think it'd need to be a more full
solution than just dj-database-url.

rtpg · 2022-11-27T12:06:37Z

Thank you, I had not seen that thread. I will do a bit more research and try to write up a proposal (EDIT: sent a message to the mailing list, waiting moderation approval)

adamchainz · 2022-11-29T04:40:50Z

django/conf/service_url.py

+    # This method may be called with an already parsed URL
+    if isinstance(url, dict):
+        return url
+    parsed = parse.urlparse(url)


You can use urlsplit instead, it’s a bit faster as it avoids unnecessary parsing

https://youtu.be/ABJvdsIANds

adamchainz · 2022-11-29T04:42:23Z

docs/ref/service-url.txt

+    from django.conf.service_url import configure_cache
+
+    CACHES = {
+       "default": configure_cache("memcached+pymemcache://localhost:11211")


Get from environ with default?

adamchainz · 2022-11-29T04:42:48Z

docs/ref/service-url.txt

+
+.. function: configure_db(value)
+
+Provide a database URL and receive a configuration dictionary meant for


Would be good to have an example of what such a URL looks like

adamchainz · 2022-11-29T04:43:07Z

docs/topics/cache.txt

Avoid noise

Suggested change

adamchainz · 2022-11-29T04:43:25Z

docs/topics/cache.txt

 them, you should stick to the cache backends included with Django. They've
 been well-tested and are well-documented.

+


Single blank line between sections ```suggestion

adamchainz · 2022-11-29T04:43:37Z

docs/topics/cache.txt

+    from django.conf.service_url import configure_cache
+
+    CACHES = {
+       "default": configure_cache("memcached+pymemcache://localhost:11211")


Use os.environ

rtpg · 2022-12-20T15:12:56Z

OK I'm closing this PR (posted my reasons here but the short version is this feels like a huge slog that I'm not really prepared to deal with, for code that has a lot of if statements that I don't know how to justify beyond "this was originally in django-database-url").

If someone else lands on this PR through Googling or whatnot... there has not been a consensus on how to do this! There is a consensus that having something would be good, but it might not be this branch.

orf and others added 5 commits November 27, 2022 11:06

Add support for database URIs into Django

9b8dafa

black formatting

75758c5

fix up imports

4714cd2

Fix up memcache tests

ba5921c

try to add more documentation

2e02476

pauloxnet reviewed Nov 27, 2022

View reviewed changes

adamchainz reviewed Nov 29, 2022

View reviewed changes

rtpg closed this Dec 20, 2022


		.. function: configure_db(value)

		Provide a database URL and receive a configuration dictionary meant for

		them, you should stick to the cache backends included with Django. They've
		been well-tested and are well-documented.

Uh oh!

Fixed #28236 - Integrate dj-database-url into Django #16331

Fixed #28236 - Integrate dj-database-url into Django #16331

Uh oh!

Conversation

rtpg commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pauloxnet commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pauloxnet Nov 27, 2022

Choose a reason for hiding this comment

Uh oh!

carltongibson commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rtpg commented Nov 27, 2022

Uh oh!

carltongibson commented Nov 27, 2022

Uh oh!

rtpg commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamchainz Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

adamchainz Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

adamchainz Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

adamchainz Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

adamchainz Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

adamchainz Nov 29, 2022

Choose a reason for hiding this comment

Uh oh!

rtpg commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rtpg commented Nov 27, 2022 •

edited

Loading

pauloxnet commented Nov 27, 2022 •

edited

Loading

carltongibson commented Nov 27, 2022 •

edited

Loading

rtpg commented Nov 27, 2022 •

edited

Loading

rtpg commented Dec 20, 2022 •

edited

Loading