Re: Calling save_m2m() in UserCreationForm.save()

2022-11-27 Thread 'Adam Johnson' via Django developers (Contributions to Django itself)
Your proposal seems reasonable - if actually saving, we should save the m2m
fields too.

I think the best next step would be to file a ticket and work on a PR. The
first step would be to add a test case reproducing the issue.

On Wed, Nov 23, 2022 at 4:59 PM Mark Gensler  wrote:

> Hello all!
>
> I noticed that when calling contrib.auth.forms.UserCreationForm.save(),
> self.save_m2m() is not called when commit=True.
>
> This caused an issue in one of my projects which uses a custom User model
> with ManyToMany fields. Is this something that should be changed? I saw in
> the docs [1] that it is advised to extend UserCreationForm and
> UserChangeForm if using a custom User model.
>
> A few reasons I see to add this step to the UserCreationForm.save() method:
>
>- UserCreationForm is a subclass of ModelForm, which does call
>save_m2m() when commit=True.
>- UserChangeForm *does* call save_m2m() as part of save(), because the
>save() method is not overloaded. This seems inconsistent!
>
> The solution I'd propose is:
>
> class UserCreationForm(forms.ModelForm):
> ...
> def save(self, commit=True):
> user = super().save(commit=False)
> user.set_password(self.cleaned_data["password1"])
> if commit:
> user.save()
> *if hasattr(self, "save_m2m"):*
> *self.save_m2m()*
> return user
>
> I'd be happy to raise a ticket and work on a patch if this change would be
> useful.
>
> Thanks
> Mark
>
> [1]
> https://docs.djangoproject.com/en/dev/topics/auth/customizing/#custom-users-and-the-built-in-auth-forms
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/c8dac66b-4a0f-4afa-b548-260fffb06e9fn%40googlegroups.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM31PTwg%2Bs-QUTjajdubSJBXf-p1f4odX89tmCzKe0%3DfaA%40mail.gmail.com.


Re: Calling save_m2m() in UserCreationForm.save()

2022-11-27 Thread Mark Gensler
Hi Adam, thanks for the reply. I'll open a ticket and start work on a PR. 
Mark

On Sunday, November 27, 2022 at 10:47:01 AM UTC Adam Johnson wrote:

> Your proposal seems reasonable - if actually saving, we should save the 
> m2m fields too.
>
> I think the best next step would be to file a ticket and work on a PR. The 
> first step would be to add a test case reproducing the issue.
>
> On Wed, Nov 23, 2022 at 4:59 PM Mark Gensler  wrote:
>
>> Hello all!
>>
>> I noticed that when calling contrib.auth.forms.UserCreationForm.save(), 
>> self.save_m2m() is not called when commit=True.
>>
>> This caused an issue in one of my projects which uses a custom User model 
>> with ManyToMany fields. Is this something that should be changed? I saw in 
>> the docs [1] that it is advised to extend UserCreationForm and 
>> UserChangeForm if using a custom User model.
>>
>> A few reasons I see to add this step to the UserCreationForm.save() 
>> method:
>>
>>- UserCreationForm is a subclass of ModelForm, which does call 
>>save_m2m() when commit=True.
>>- UserChangeForm *does* call save_m2m() as part of save(), because 
>>the save() method is not overloaded. This seems inconsistent!
>>
>> The solution I'd propose is:
>>
>> class UserCreationForm(forms.ModelForm):
>> ...
>> def save(self, commit=True):
>> user = super().save(commit=False)
>> user.set_password(self.cleaned_data["password1"])
>> if commit:
>> user.save()
>> *if hasattr(self, "save_m2m"):*
>> *self.save_m2m()*
>> return user
>>
>> I'd be happy to raise a ticket and work on a patch if this change would 
>> be useful.
>>
>> Thanks
>> Mark
>>
>> [1] 
>> https://docs.djangoproject.com/en/dev/topics/auth/customizing/#custom-users-and-the-built-in-auth-forms
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/c8dac66b-4a0f-4afa-b548-260fffb06e9fn%40googlegroups.com
>>  
>> 
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e96fff49-becc-476c-a7fc-5b4c00c7b8d7n%40googlegroups.com.


Re: Combining multiple aggregations with annotate()

2022-11-27 Thread Yonas
Hi Simon,

In addition to what's documented, that's another way annotate() gives the 
wrong result.

There are a lot of duplicates of #10060, and there are a lot of people who 
run into this issue. 

Given the age of the ticket and the lack of a simple backward compatible 
solution, what would be the way forward? Maybe breaking changes? Or 
updating the docs to show a safe usage of annotate()? Or throwing an error?

On Wednesday, January 5, 2022 at 7:44:11 PM UTC+3 charettes wrote:

> While addressing the issue is complex and will require quite a bit of work 
> to solve in a backward-compatible way I believe that it should be feasible 
> to emit a MultipleMultiValuedJoin(RuntimeWarning) with pointers for 
> alternatives (e.g. using subqueries) without too much hassle when 
> aggregation is performed and more than one multi-valued relationship is 
> involved.
>
> The docs you pointed at mention the multiple aggregation problem but a 
> similar thing happens when filtering against a multi-valued relationship 
> after a single aggregation[0] so the problem can be generalized to any form 
> of aggregation on a query that joins more than one multi-valued 
> relationship (n-to-many).
>
> Simon
>
> [0] https://code.djangoproject.com/ticket/33403
>
>
>
> Le mercredi 5 janvier 2022 à 09:59:24 UTC-5, Yonas a écrit :
>
>> In that case, at least there should be a warning message in the 
>> documentation. And what do you think of the example? Isn't it contradicting 
>> the documentation?
>>
>> On Wednesday, January 5, 2022 at 5:16:43 PM UTC+3 niccol...@gmail.com 
>> wrote:
>>
>>> I would be in favor of a real time information about the issue.
>>> Il giorno mercoledì 5 gennaio 2022 alle 15:13:17 UTC+1 Yonas ha scritto:
>>>
 Hello,

 There's a ticket  opened 
 13 years ago explaining a problem with combining multiple aggregations 
 with 
 annotate(). And the solution appears to be documenting 
 
  the 
 problem.

 But for people skimming through the documentation, the message might 
 not be noticeable. Showing the problem in a warning message could help 
 draw 
 attention better. It's used here 
  and 
 in other places in the doc.

 In addition to documenting the problem, raising an exception might 
 prevent developers from spending hours trying to debug their code.

 While the problem is recognized, there's an example  
 in
  
 the documentation that shows the usage of multiple aggregations.

>>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4fec3010-2adf-4e1c-be85-4b66e9f20982n%40googlegroups.com.


Re: Using comment in Django

2022-11-27 Thread Tawfeeq Zaghlool
Thank you, Adam.
I really appreciate your help.

Best 

On Saturday, November 26, 2022 at 10:16:45 PM UTC+3 Adam Johnson wrote:

> Hi!
>
> I think you've found the wrong mailing list for this post. This mailing 
> list is for discussing the development of Django itself, not for support 
> using Django. This means the discussions of bugs and features in Django 
> itself, rather than in your code using it. People on this list are unlikely 
> to answer your support query with their limited time and energy.
>
> For support, please follow the "Getting Help" page: 
> https://docs.djangoproject.com/en/stable/faq/help/ . This will help you 
> find people who are willing to support you, and to ask your question in a 
> way that makes it easy for them to answer.
>
> Thanks for your understanding and all the best,
>
> Adam
>
> On Sat, Nov 26, 2022 at 10:55 AM Tawfeeq Zaghlool  
> wrote:
>
>> I have two models Profile and Post; I want to allow the user to add 
>> comments on any of them without repeating the code. 
>> In other words, the Profile and the Post components contain a comment 
>> field, and the user who views the Profile wants to add comments, or who 
>> views Posts intends to add a comment.
>> I have created another component (class) for the comments and assigned 
>> the comments as a foreign key inside Profile and Post.
>>
>> Do I have another solution?
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/20f8929b-59a5-4379-beb1-def34419682bn%40googlegroups.com
>>  
>> 
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a84fdc45-24c1-4fdd-85ea-3b33da0ac643n%40googlegroups.com.


Potential way forward for DATABASE_URL

2022-11-27 Thread Raphael G


Some base industry background. It's a pretty common convention to share 
credentials in environment variables. For many PaaS, it's common to use 
connection URLs to do so. So DATABASE_URL will have a URL like 
postgres://my_user:mypassword@somedomain/database stuffed into a single 
environment variable.

Django expects a configuration dictionary for its drivers. So what do 
people do? People install django-database-url, and pass in the string into 
that library (or rather, the library will read a blessed environment 
variable). Absent that they'll need to manually parse out the information 
and build the configuration dictionary. So if you just have Django time to 
futz around with urlparse

There have been some discussions about how to make this better, including:

   - adopting dj-database-url as an "official django project" via DEP 7 
   
   - instead adopt a behavior where Django will automatically handle a 
   string as an alternative to a configuration dictionary 
    (this is discussed in 
   the first link as PR 10786)
   - Integrate dj-database-url but expand on it a bit to also help with 
   cache configuration, and have backends handle URL parsing 
   

These haven't seen much movement in the past couple of years. A comment in 
one of these e-mail threads:

> 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.

dj-database-url takes something from an environment variable and provides a 
configuration dictionary. There's this feeling that having Django directly 
accept a string would feel more natural and correct. There are also other 
libraries like dsnparse, and people proposing things like adding a DSN name 
into settings.

I think of all the options, the third option (the proposal by Tom Forbes) 
is a very good option. What it looks like in practice is the addition of 
the following:

   - the ability for database backends to register protocol names for URLs, 
   so that postgres://localhost:5432 will properly map to the 
   django.db.backends.postgresql backend, but people can show up with their 
   own mappings.
   - A configure_db(url) function, that will return a configuration 
   dictionary meant for DATABASES
   - A similar configure_cache(url) function that will give cache 
   configuration dictionaries meant for CACHES

A thing that is notably absent here is any blessing of DATABASE_URL. You 
have to do

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

yourself. It's not "ideal" in that you don't magically get behavior from 
your URLs, but that also means you're just doing something in a 
straightforward way that should be easy to debug with some print statements 
when needed. It feels way less likely for this to be a major design miss.

The motivating examples for the two above being supported is that Heroku 
will provide DATABASE_URL and REDIS_URL.

The nice thing about this solution is that it doesn't block future design 
space. We get a configuration dictionary that matches the existing 
functionality, because the added API is simple it's easy for people to 
inspect the results, and of course it doesn't preclude people from keeping 
on with their existing solutions. There isn't even an assumed usage of 
DATABASE_URL like with dj-database-url! Mostly magic free.

I tried to rebase the PR 
 including 
the above functionality from a couple years ago, and added some basic 
documentation. This doesn't try and convince users to use this, but I 
believe the usage would be sufficient for simple cases.

So my ask here: how do people feel about moving forward with this limited 
scope? Previous discussions talked about wanting a larger scope for it to 
get merged into core. I believe that instead targetting a smaller scope 
means we can at least provide a workable answer to the DATABASE_URL question 
in the near term. And when consensus coalesces around a good overall answer 
to settings, the actual URL parsing logic will already be present and even 
more battle tested.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d990fef4-586b-447f-afd1-b53f23237a3an%40googlegroups.com.


Re: Integrate dj-database-url into Django

2022-11-27 Thread Raphael G
Alright, I tried to revive Tom Forbe's work on this in  
https://github.com/django/django/pull/16331

My honest feeling here is that if Django existed just for me, I would 
really just want to get this pulled in as an option, and trying to increase 
the scope beyond "given a URL, get a dictionary of settings" is a tarpit 
that is not worth venturing into in a first merge. Though it's definitely a 
tarpit worth venturing into as a future step! 

So my strategy here was to try and get this as close to mergeable as 
possible, with as few points of discussion needed as possible. I tried to 
document what Tom did, more or less disregarded the idea of providing DSN 
support in settings, and for memcache avoided blessing an "official" 
backend. The main thing should be that if people google Django DATABASE_URL 
there should be a good answer in the docs, IMO.

As someone who's dealt with custom backends in various parts of Django, and 
with libs, I think Tom's simple registry pattern for custom protocols is 
more than sufficient for library writers. 

Documentation is minimal, but exists. While I basically expect people 
writing custom DB backends or cache backends to directly look at the 
source, at the very least we are pointing to the existence of functions for 
registration in the docs in this PR.

I'm sure we all have these simple projects that really don't need to do 
much, we all have been pulling in dj_database_url on the side to get this 
and feeling a bit off about it, so let's try and get that use case handled 
in a way that doesn't prevent further niceness in the future! 

Raphael
On Monday, December 24, 2018 at 7:01:23 AM UTC+9 Raffaele Salmaso wrote:

> Hi all,
> I'm working on https://github.com/django/django/pull/10786 (which is a 
> port of https://pypi.org/project/django-service-urls/ , which is a 
> 'fork/rewrite' of Tom PR).
> I need to (re)read all these emails to find ideas to improve the 
> PR/package.
>
> On Sat, Jul 28, 2018 at 9:44 PM Tom Forbes  wrote:
>
>> So in the PR I proposed I only bits I took verbatim from dj-database-url 
>> are the tests. The rest is re-implemented. I think it's a pretty good POC 
>> but I haven't touched it in a while.
>>
>> In any case we have to implement our own parsing for backends that do not 
>> support passing in a URL to the connection library. 
>>
>> Making postgres skip our parsing step and passing it in directly is an 
>> implementation detail and there are much more important questions around 
>> the API design to answer before this has any chance of being included.
>>
>> On Sat, 28 Jul 2018, 12:57 Maciej Urbański,  wrote:
>>
>>> I would agree that DSN support seems like a nicer alternative to just 
>>> copying dj-database-url, because it not only focuses on 12factor 
>>> configuration in enviroment variables, but also enables some additional 
>>> flexibility for the database connection option passing.
>>>
>>> As for 12factor, I think https://gist.github.com/telent/9742059 is a 
>>> good read as to why exposing as enviroment variables maybe not the best 
>>> motivation. Having to accommodate settings, like database connection 
>>> information, just so they can be fitted into single string put conveyable 
>>> by enviroment variable is a case in point. We likely can do the same for 
>>> Cache addresses as mentioned previously, although we may end up inventing 
>>> new URI schemes do to that.., but django overall has multitude of other 
>>> options that are not as easy to stringify.
>>>
>>> On Friday, 27 July 2018 19:14:12 UTC+2, gw...@fusionbox.com wrote:

 I'd like to approach this as 'support database urls in django', rather 
 than 'copy/paste dj-database-url into django'. For postgres (I'm not sure 
 about other backends), a database url can be passed directly to psycopg2. 
 The postgres connection string format actually supports more features than 
 is possible with django's HOST/USER/PORT... style settings. For example, 
 you can pass multiple hosts and psycopg2 will attempt to connect to one of 
 them: https://paquier.xyz/postgresql-2/postgres-10-multi-host-connstr/. 
 Any attempt to parse the url in django will result in a loss of those 
 features.

 The only problem I see is that we have to parse the database backend 
 out of the url, and you can't pass a url like 'postgis://' to 
 psyscopg2. I'd like to be able to do something like:

 DATABASES = {
 'default': {
 'DSN': 'postgres://',
 'ENGINE': 'django.contrib.gis.db.backends.postgis',
 },
 }

 And let psycopg2 handle the DSN.

>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-develop...@googlegroups.com.
>>> To post to this group, send email to django-d...@googlegroups

Re: Potential way forward for DATABASE_URL

2022-11-27 Thread Jörg Breitbart

Am 27.11.22 um 13:51 schrieb Raphael G:
So my ask here: how do people feel about moving forward with this 
limited scope? Previous discussions talked about wanting a larger scope 
for it to get merged into core. I believe that instead targetting a 
smaller scope means we can at least provide a workable answer to 
theDATABASE_URLquestion in the near term. And when consensus coalesces 
around a good overall answer to settings, the actual URL parsing logic 
will already be present and even more battle tested.


+1 from my side, even with reduced versatility or scope it covers 
(sometimes a too broad scope just gives you more pros and cons 
discussions with a higher probability of sidetracking things into nowhere).


Some background - several years back we used SQLObject as ORM driver 
alot in python projects and switched later to SQLAlchemy (still using it 
for heavy db lifting projects). I always found the URI connection scheme 
appealing and easy to go with.


On django side of things I kinda always wondered why the connection 
settings were that verbose to get running, but never wondered hard 
enough to question its explicit style.


Well, in terms of conformance to other frameworks I see a clear benefit 
here.


Cheers,
Jörg

--
You received this message because you are subscribed to the Google Groups "Django 
developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/bc54e939-df5a-b370-ca21-d8415f274868%40netzkolchose.de.


Re: Potential way forward for DATABASE_URL

2022-11-27 Thread 'Tobias McNulty' via Django developers (Contributions to Django itself)
Hi Raphael,

Thanks for taking this on.

Starting with a limited scope seems like a good idea to me.

A couple other things I like about this approach:

- It tackles cache URLs at the same time (it makes sense for them to mirror
one another, IMO).
- No implicit usage of DATABASE_URL, but as you said it still supplies an
easily searchable answer for "Django DATABASE_URL."

Cheers,
Tobias

On Sun, Nov 27, 2022, 2:40 PM Raphael G  wrote:

> Some base industry background. It's a pretty common convention to share
> credentials in environment variables. For many PaaS, it's common to use
> connection URLs to do so. So DATABASE_URL will have a URL like
> postgres://my_user:mypassword@somedomain/database stuffed into a single
> environment variable.
>
> Django expects a configuration dictionary for its drivers. So what do
> people do? People install django-database-url, and pass in the string
> into that library (or rather, the library will read a blessed environment
> variable). Absent that they'll need to manually parse out the information
> and build the configuration dictionary. So if you just have Django time
> to futz around with urlparse
>
> There have been some discussions about how to make this better, including:
>
>- adopting dj-database-url as an "official django project" via DEP 7
>
> 
>- instead adopt a behavior where Django will automatically handle a
>string as an alternative to a configuration dictionary
> (this is discussed in
>the first link as PR 10786)
>- Integrate dj-database-url but expand on it a bit to also help with
>cache configuration, and have backends handle URL parsing
>
> 
>
> These haven't seen much movement in the past couple of years. A comment in
> one of these e-mail threads:
>
> > 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.
>
> dj-database-url takes something from an environment variable and provides
> a configuration dictionary. There's this feeling that having Django
> directly accept a string would feel more natural and correct. There are
> also other libraries like dsnparse, and people proposing things like
> adding a DSN name into settings.
>
> I think of all the options, the third option (the proposal by Tom Forbes)
> is a very good option. What it looks like in practice is the addition of
> the following:
>
>- the ability for database backends to register protocol names for
>URLs, so that postgres://localhost:5432 will properly map to the
>django.db.backends.postgresql backend, but people can show up with
>their own mappings.
>- A configure_db(url) function, that will return a configuration
>dictionary meant for DATABASES
>- A similar configure_cache(url) function that will give cache
>configuration dictionaries meant for CACHES
>
> A thing that is notably absent here is any blessing of DATABASE_URL. You
> have to do
>
> DATABASES = {
> 'default': configure_db(os.environ['DATABASE_URL'])
> }
>
> yourself. It's not "ideal" in that you don't magically get behavior from
> your URLs, but that also means you're just doing something in a
> straightforward way that should be easy to debug with some print statements
> when needed. It feels way less likely for this to be a major design miss.
>
> The motivating examples for the two above being supported is that Heroku
> will provide DATABASE_URL and REDIS_URL.
>
> The nice thing about this solution is that it doesn't block future design
> space. We get a configuration dictionary that matches the existing
> functionality, because the added API is simple it's easy for people to
> inspect the results, and of course it doesn't preclude people from keeping
> on with their existing solutions. There isn't even an assumed usage of
> DATABASE_URL like with dj-database-url! Mostly magic free.
>
I tried to rebase the PR
>  
> including
> the above functionality from a couple years ago, and added some basic
> documentation. This doesn't try and convince users to use this, but I
> believe the usage would be sufficient for simple cases.
>
> So my ask here: how do people feel about moving forward with this limited
> scope? Previous discussions talked about wanting a larger scope for it to
> get merged into core. I believe that instead targetting a smaller scope
> means we can at least provide a workable answer to the DATABASE_URL question
> in the near term. And when consensus coalesces around a good overall answer
> to settings, the actual URL parsing logic will already be present and ev

Async wrappers in contrib packages

2022-11-27 Thread Jon Janzen
Hey everyone,

Sorry if I'm not following correct protocol on this or if this has already 
been discussed elsewhere, but is there any consensus about (or needed for) 
creating async versions of contrib packages?

My personal interest in this is about django.contrib.auth (login, 
authenticate, etc.) and django.contrib.syndication (views.Feed) but I would 
guess this sort of work would fall under a general policy.

I read DEP009 
 and 
didn't see any discussion of this topic, nor could I find any discussions 
on the ticket tracker (my skills using the tracker are limited). I could 
only find 2 files mentioning "async" in django/contrib/ (git grep "async" 
django/contrib | grep "\.py:"):

* django/contrib/contenttypes/fields.py (added by me recently 
,
 
a bug fix )
* django/contrib/staticfiles/handlers.py (added as part of standing up 
async support 

)

Has there been any consensus about this? If I'm interested in async 
versions of functions/features in contrib packages should I just file 
tickets, or is this something that might need a DEP first?

Again, sorry if I'm barking up the wrong tree. It's not my intention to 
waste anyone's time!

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/70e80f26-e1fa-4aa9-a3a8-0d7dc38752c3n%40googlegroups.com.


Re: Integrate dj-database-url into Django

2022-11-27 Thread Carlton Gibson
Hi Raphael, thanks for picking this up.

Looking at the history here, particularly the discussion here
https://groups.google.com/g/django-developers/c/UQjpzB39JN0/m/XGqdV8nbBwAJ
but there are others, the reason this hasn't already happened is that if
we're going to merge this into core it would be nice (required?) to handle
extra cases than (to paraphrase) "just what Heroku expects".

It doesn't look like anyone has sat down to spec out exactly what that
means, but there seem to be plenty of thoughts in the discussion.  ... 🤔

JKM's comment on the thread there is representative of the general thought:

> 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.

The trouble with merging a suboptimal solution is that once it's in, it's *very
hard* to change. We have the whole API stability policy to contend with
(and rightly so).
We bring in a half-baked solution; likely "recommend" it (for some value of
that) and then face a long stream of difficult adjustments whilst we try
and accommodate the necessary adjustments.

Given that dj-database-url is stable, a single line install, and (most
importantly) not being developed to try and accommodate the issues that
stopped it just being merged into core already, I think it should continue
to live as a third-party package.

The place for the development to happen is in dj-database-url or a
successor package. (The first step would be a Roadmap drawn from the
historical discussion.)
Once (if) that gets to the point where it clearly addresses the 90% of
needs, then there's a case for adding it to core. I don't think we can just
push forward given that nothing changed in the last few years.

Kind Regards,

Carlton



On Sun, 27 Nov 2022 at 20:40, Raphael G  wrote:

> Alright, I tried to revive Tom Forbe's work on this in
> https://github.com/django/django/pull/16331
>
> My honest feeling here is that if Django existed just for me, I would
> really just want to get this pulled in as an option, and trying to increase
> the scope beyond "given a URL, get a dictionary of settings" is a tarpit
> that is not worth venturing into in a first merge. Though it's definitely a
> tarpit worth venturing into as a future step!
>
> So my strategy here was to try and get this as close to mergeable as
> possible, with as few points of discussion needed as possible. I tried to
> document what Tom did, more or less disregarded the idea of providing DSN
> support in settings, and for memcache avoided blessing an "official"
> backend. The main thing should be that if people google Django DATABASE_URL
> there should be a good answer in the docs, IMO.
>
> As someone who's dealt with custom backends in various parts of Django,
> and with libs, I think Tom's simple registry pattern for custom protocols
> is more than sufficient for library writers.
>
> Documentation is minimal, but exists. While I basically expect people
> writing custom DB backends or cache backends to directly look at the
> source, at the very least we are pointing to the existence of functions for
> registration in the docs in this PR.
>
> I'm sure we all have these simple projects that really don't need to do
> much, we all have been pulling in dj_database_url on the side to get this
> and feeling a bit off about it, so let's try and get that use case handled
> in a way that doesn't prevent further niceness in the future!
>
> Raphael
> On Monday, December 24, 2018 at 7:01:23 AM UTC+9 Raffaele Salmaso wrote:
>
>> Hi all,
>> I'm working on https://github.com/django/django/pull/10786 (which is a
>> port of https://pypi.org/project/django-service-urls/ , which is a
>> 'fork/rewrite' of Tom PR).
>> I need to (re)read all these emails to find ideas to improve the
>> PR/package.
>>
>> On Sat, Jul 28, 2018 at 9:44 PM Tom Forbes  wrote:
>>
>>> So in the PR I proposed I only bits I took verbatim from dj-database-url
>>> are the tests. The rest is re-implemented. I think it's a pretty good POC
>>> but I haven't touched it in a while.
>>>
>>> In any case we have to implement our own parsing for backends that do
>>> not support passing in a URL to the connection library.
>>>
>>> Making postgres skip our parsing step and passing it in directly is an
>>> implementation detail and there are much more important questions around
>>> the API design to answer before this has any chance of being included.
>>>
>>> On Sat, 28 Jul 2018, 12:57 Maciej Urbański,  wrote:
>>>
 I would agree that DSN support seems like a nicer alternative to just
 copying dj-database-url, because it not only focuses on 12factor
 configuration in enviroment variables, but also enables some additional
 flexibility for the database connection option passing.

 As for 12factor, I think https://gist.github.com/telent/9742059 is a
 good r