Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True - Mailing list pgsql-www

From Magnus Hagander
Subject Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
Date
Msg-id CABUevEyBc9+zspBNE_TgKwtURKeuAsf++Wgszb8rnpDu=KPdKQ@mail.gmail.com
Whole thread Raw
In response to [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True  (Célestin Matte <celestin.matte@cmatte.me>)
Responses Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True  (Célestin Matte <celestin.matte@cmatte.me>)
List pgsql-www
On Fri, Jan 28, 2022 at 6:37 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
>
> pglister_sync.py is a script used to synchronize things between pglister and pgarchives: lists, subscribers etc.
>
> subscriber_access is not set in pglister_sync's query, and is set to null=False in Django's model. As a consequence,
pglister_syncfails to add new lists: 
> Traceback (most recent call last):
>   File "/srv/pgarchives/local/loader/pglister_sync.py", line 68, in <module>
>     'groupname': l['group']['groupname'],
> psycopg2.errors.NotNullViolation: null value in column "subscriber_access" violates not-null constraint
> DETAIL:  Failing row contains (8, test-pglister-sync, test-pglister-sync, , t, 1, null).
>
> I don't know if there is a way to configure postgres to use the default value , but I think it would be wiser to
explicitlyset this variable in pglister_sync.py. 

Interesting. Yes there is, set the columns default to false. Which is
what is running on both our production servers for postgresql.org --
but I'm unsure how it actually got there given that Django is dumb
enough not to propagate that down to the database, assuming all access
will forever be through the ORM. We must've done some manual step when
deploying that, that has since been forgotten.

This should definitely be fixed.

> By default, subscriber_access is set to False and there is no way to modify that within the web interface.
> As a consequence, access to lists on private servers is restricted to superusers, and there is no easier way to
modifythat than to edit the database manually. 
>
> It seems more logical to me that this value be set to True by default, as access can still be moderated to avoid
listsbeing publicly available. 

In what way would access "still be moderated"? In pgarchives, that's a
pure boolean and there are no further checks. User accounts are
auto-created.

The idea is that anything that's "open" should have to be set
explicitly and thus we should default to it being off. Based on that I
have at least initially applied a version of your patch that sets it
to false.


> That said, it may be better to have a way to modify that within the web interface in pglister.

I agree in principle. The argument does fall off a bit on the fact
that there is *no* admin interface to pgarchives. You don't have a way
to add a list manually either, without doing it directly in SQL. So we
either accept that SQL is the way things are done, or we should tackle
the bigger problem of setting up such an interface. But I think we
could get pretty far by just enabling the general django admin
interface and set up the required classes for that -- we don't
necessarily need to move things like reparsing and hiding of messages
into such an admin interface.

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/



pgsql-www by date:

Previous
From: Célestin Matte
Date:
Subject: [PATCH] pgarchives: Bugfix: missing ids in pglister_sync
Next
From: Magnus Hagander
Date:
Subject: Re: [PATCH] pgarchives: Bugfix: missing ids in pglister_sync