public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
8+ messages / 2 participants
[nested] [flat]
* [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
@ 2022-01-28 17:37 Célestin Matte <[email protected]>
2022-01-28 17:43 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-30 12:20 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Célestin Matte @ 2022-01-28 17:37 UTC (permalink / raw)
To: PostgreSQL WWW <[email protected]>
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_sync fails 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 explicitly set this variable in pglister_sync.py.
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 modify that 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 lists being publicly available.
That said, it may be better to have a way to modify that within the web interface in pglister.
--
Célestin Matte
Attachments:
[text/x-patch] 0001-pglister_sync-import-lists-with-subscriber_access-se.patch (1.2K, 2-0001-pglister_sync-import-lists-with-subscriber_access-se.patch)
download | inline diff:
From 6e93f0e3d5ae297a3ce61e238708b7235acf9084 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 1 Nov 2021 20:43:51 +0100
Subject: [PATCH] pglister_sync: import lists with subscriber_access set to
True
Otherwise, there is no in-app way to set it
---
loader/pglister_sync.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/loader/pglister_sync.py b/loader/pglister_sync.py
index f95d369..dd5bec4 100755
--- a/loader/pglister_sync.py
+++ b/loader/pglister_sync.py
@@ -60,7 +60,7 @@ if __name__ == "__main__":
'name': l['listname'],
})
if curs.rowcount == 0:
- curs.execute("INSERT INTO lists (listname, shortdesc, description, active, groupid) SELECT %(name)s, %(name)s, %(desc)s, 't', groupid FROM listgroups WHERE groupname=%(groupname)s RETURNING listid, listname", {
+ curs.execute("INSERT INTO lists (listname, shortdesc, description, active, groupid, subscriber_access) SELECT %(name)s, %(name)s, %(desc)s, 't', groupid, 't' FROM listgroups WHERE groupname=%(groupname)s RETURNING listid, listname", {
'name': l['listname'],
'desc': l['longdesc'],
'groupname': l['group']['groupname'],
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
@ 2022-01-28 17:43 ` Célestin Matte <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Célestin Matte @ 2022-01-28 17:43 UTC (permalink / raw)
To: [email protected]
Sorry, accidentally sent unfinished email containing contradictory information... Please ignore the previous one.
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_sync fails 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 explicitly set this variable in pglister_sync.py (attached patch in previous email)
It seems more logical to me to set this value to True, as otherwise access to lists on private servers is restricted to superusers, and there is no easier way to modify that than to edit the database manually. But then, the model may need to be update to have this value set to default=True as well. What do you think?
That said, it may be better to have a way to modify that within the web interface in pglister.
--
Célestin Matte
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
@ 2022-01-30 12:20 ` Magnus Hagander <[email protected]>
2022-02-02 08:16 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Magnus Hagander @ 2022-01-30 12:20 UTC (permalink / raw)
To: Célestin Matte <[email protected]>; +Cc: PostgreSQL WWW <[email protected]>
On Fri, Jan 28, 2022 at 6:37 PM Célestin Matte <[email protected]> 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_sync fails 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 explicitly set 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 modify that 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 lists being 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/
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-30 12:20 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
@ 2022-02-02 08:16 ` Célestin Matte <[email protected]>
2022-02-05 17:24 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Célestin Matte @ 2022-02-02 08:16 UTC (permalink / raw)
To: Magnus Hagander <[email protected]>; +Cc: PostgreSQL WWW <[email protected]>
> This should definitely be fixed.
Thanks!
>> 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 modify that 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 lists being 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.
I meant that subscriptions can still be moderated in pglister (if lists are configured that way), so that anybody does not have access to archives.
> 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 makes sense.
>> 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.
I meant this could be added in the admin interface of pglister, not pgarchives, as it already exists and pglister_sync can then push (and update) the configuration to pgarchives.
--
Célestin Matte
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-30 12:20 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
2022-02-02 08:16 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
@ 2022-02-05 17:24 ` Magnus Hagander <[email protected]>
2023-02-28 10:39 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2024-02-12 20:47 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Magnus Hagander @ 2022-02-05 17:24 UTC (permalink / raw)
To: Célestin Matte <[email protected]>; +Cc: PostgreSQL WWW <[email protected]>
On Wed, Feb 2, 2022 at 9:16 AM Célestin Matte <[email protected]> wrote:
>
> > This should definitely be fixed.
>
> Thanks!
>
> >> 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 modify that 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 lists being 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.
>
> I meant that subscriptions can still be moderated in pglister (if lists are configured that way), so that anybody does not have access to archives.
Ah, right. That still doesn't fully solve the problem though --
because once somebody has been approved in moderation, they
automatically get access to everything on the list from before they
were added. Which may not be what's wanted -- we have cases where
we're archiving things for "no regular use", but leave it for
superusers to be able to go look things up in an emergency for
example. In that usecase, it's not tied to the subscription at all.
> > 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 makes sense.
>
> >> 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.
>
> I meant this could be added in the admin interface of pglister, not pgarchives, as it already exists and pglister_sync can then push (and update) the configuration to pgarchives.
Ah, then I understand.
That's an interesting aspect. Right now, pglister has no knowledge of
what actually runs on the archiving side other than "send the emails
over here" and "create links that look like this".
In general, I like that level of disconnect -- it should be possible
to swap out the archive solution from underneath it, or indeed run the
same pglister instance against multiple different types of archives.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-30 12:20 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
2022-02-02 08:16 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-02-05 17:24 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
@ 2023-02-28 10:39 ` Célestin Matte <[email protected]>
2023-04-03 15:16 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Célestin Matte @ 2023-02-28 10:39 UTC (permalink / raw)
To: Magnus Hagander <[email protected]>; PostgreSQL WWW <[email protected]>
Attached another proposed solution to that problem, with a series of patches for pglister and pgarchives:
- 0001-Add-subscriber_access-field-to-ListGroup_pglister.patch adds a subscriber_access field in pglister's admin section
- 0002-Add-subscriber_access-to-archives-API_pglister.patch adds subscriber_access to the API's archive section in pglister
- 0001-pglister_sync-obtain-subscriber_access-from-API_pgarchives.patch uses the value received from the API instead of a default "False" that has to be manually changed
As a reminder, the problem was that subscriber_access was set to False by default, which means that any list created on pglister and set to be archived on pgarchives won't be reachable by subscribers, unless the subscriber_access field is manually modified in the database.
--
Célestin Matte
Attachments:
[text/x-patch] 0001-Add-subscriber_access-field-to-ListGroup_pglister.patch (1.8K, 2-0001-Add-subscriber_access-field-to-ListGroup_pglister.patch)
download | inline diff:
From 872c5cee537a6c387ad9b0de97694fb8fa12f78b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Thu, 23 Feb 2023 18:07:28 +0100
Subject: [PATCH 1/2] Add subscriber_access field to ListGroup
---
.../migrations/0055_auto_20230223_1706.py | 18 ++++++++++++++++++
web/pglister/lists/models.py | 2 ++
2 files changed, 20 insertions(+)
create mode 100644 web/pglister/lists/migrations/0055_auto_20230223_1706.py
diff --git a/web/pglister/lists/migrations/0055_auto_20230223_1706.py b/web/pglister/lists/migrations/0055_auto_20230223_1706.py
new file mode 100644
index 0000000..bdb4ede
--- /dev/null
+++ b/web/pglister/lists/migrations/0055_auto_20230223_1706.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.24 on 2023-02-23 17:06
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('lists', '0054_add_archivedat'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='listgroup',
+ name='subscriber_access',
+ field=models.BooleanField(default=False, help_text='Can subscribers get full access to the archives?', null=True),
+ ),
+ ]
diff --git a/web/pglister/lists/models.py b/web/pglister/lists/models.py
index dba9689..cf3ffec 100644
--- a/web/pglister/lists/models.py
+++ b/web/pglister/lists/models.py
@@ -111,6 +111,8 @@ class Domain(models.Model):
class ListGroup(models.Model):
groupname = models.CharField(max_length=100, null=False, blank=False)
sortkey = models.IntegerField(null=False, default=10)
+ subscriber_access = models.BooleanField(null=True, blank=False, default=False,
+ help_text="Can subscribers get full access to the archives?")
def __str__(self):
return self.groupname
--
2.39.2
[text/x-patch] 0001-pglister_sync-obtain-subscriber_access-from-API_pgarchives.patch (1.5K, 3-0001-pglister_sync-obtain-subscriber_access-from-API_pgarchives.patch)
download | inline diff:
From ff71fc36609ef139837612c5d60906edcd340b63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 27 Feb 2023 19:22:33 +0100
Subject: [PATCH] pglister_sync: obtain subscriber_access from API
---
loader/pglister_sync.py | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/loader/pglister_sync.py b/loader/pglister_sync.py
index d7e1925..7440ab8 100755
--- a/loader/pglister_sync.py
+++ b/loader/pglister_sync.py
@@ -42,15 +42,16 @@ if __name__ == "__main__":
obj = r.json()
# For groups, just add them if they don't exist
- groups = {g['group']['id']: g['group']['groupname'] for g in obj}
+ groups = {g['group']['id']: (g['group']['groupname'], g['group']['subscriber_access']) for g in obj}
- for id, name in list(groups.items()):
+ for id, (name, subscriber_access) in list(groups.items()):
curs.execute("SELECT EXISTS (SELECT 1 FROM listgroups WHERE groupname=%(group)s)", {
'group': name,
})
if not curs.fetchone()[0]:
- curs.execute("INSERT INTO listgroups (groupname, sortkey) VALUES (%(group)s, 100) RETURNING groupname", {
+ curs.execute("INSERT INTO listgroups (groupname, sortkey, subscriber_access) VALUES (%(group)s, 100, %(subscriber_access)s) RETURNING groupname", {
'group': name,
+ 'subscriber_access': subscriber_access,
})
print("Added group %s" % name)
--
2.39.2
[text/x-patch] 0002-Add-subscriber_access-to-archives-API_pglister.patch (2.8K, 4-0002-Add-subscriber_access-to-archives-API_pglister.patch)
download | inline diff:
From 6e94a2380bcb471e1a089a61931e4d9d16e80752 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 27 Feb 2023 19:07:05 +0100
Subject: [PATCH 2/2] Add subscriber_access to archives API
---
web/pglister/lists/views_api.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/web/pglister/lists/views_api.py b/web/pglister/lists/views_api.py
index 24e41df..50de9c8 100644
--- a/web/pglister/lists/views_api.py
+++ b/web/pglister/lists/views_api.py
@@ -140,11 +140,11 @@ class ArchivesApi(View):
def get_lists(self, with_subscribers):
curs = connection.cursor()
if with_subscribers:
- curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname) AS group, COALESCE(array_agg(u.username) FILTER (WHERE u.username IS NOT NULL), ARRAY[]::text[]) AS subscribers FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id LEFT JOIN mailinglist_subscribers s ON s.listid=l.id LEFT JOIN auth_user u ON u.id=s.userid WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
+ curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname, 'subscriber_access', g.subscriber_access) AS group, COALESCE(array_agg(u.username) FILTER (WHERE u.username IS NOT NULL), ARRAY[]::text[]) AS subscribers FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id LEFT JOIN mailinglist_subscribers s ON s.listid=l.id LEFT JOIN auth_user u ON u.id=s.userid WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
'archiveid': self.archiveserver.id,
})
else:
- curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname) AS group FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
+ curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname, 'subscriber_access', g.subscriber_access) AS group FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
'archiveid': self.archiveserver.id,
})
columns = [col[0] for col in curs.description]
--
2.39.2
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-30 12:20 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
2022-02-02 08:16 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-02-05 17:24 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
2023-02-28 10:39 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
@ 2023-04-03 15:16 ` Célestin Matte <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Célestin Matte @ 2023-04-03 15:16 UTC (permalink / raw)
To: [email protected]
Update: previously attached patches were incorrectly using subscriber_access in ListGroup, although it's a field of List in pgarchives.
Besides, they were not built on top of upstream. Please note that this set of patches is built on top of the patch I sent in gitlab [1], as upstream currently crashes on migration.
New sets of patched attached.
[1] https://gitlab.com/pglister/pglister/-/merge_requests/33
On 28/02/2023 11:39, Célestin Matte wrote:
> Attached another proposed solution to that problem, with a series of patches for pglister and pgarchives:
> - 0001-Add-subscriber_access-field-to-ListGroup_pglister.patch adds a subscriber_access field in pglister's admin section
> - 0002-Add-subscriber_access-to-archives-API_pglister.patch adds subscriber_access to the API's archive section in pglister
> - 0001-pglister_sync-obtain-subscriber_access-from-API_pgarchives.patch uses the value received from the API instead of a default "False" that has to be manually changed
>
> As a reminder, the problem was that subscriber_access was set to False by default, which means that any list created on pglister and set to be archived on pgarchives won't be reachable by subscribers, unless the subscriber_access field is manually modified in the database.
--
Célestin Matte
Attachments:
[text/x-patch] 0001-Add-subscriber_access-field-to-List.patch (1.9K, 2-0001-Add-subscriber_access-field-to-List.patch)
download | inline diff:
From 82d233cbd4d94f1cf7376aff35ec53737bc650a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 3 Apr 2023 17:07:52 +0200
Subject: [PATCH 1/2] Add subscriber_access field to List
---
.../migrations/0058_list_subscriber_access.py | 18 ++++++++++++++++++
web/pglister/lists/models.py | 2 ++
2 files changed, 20 insertions(+)
create mode 100644 web/pglister/lists/migrations/0058_list_subscriber_access.py
diff --git a/web/pglister/lists/migrations/0058_list_subscriber_access.py b/web/pglister/lists/migrations/0058_list_subscriber_access.py
new file mode 100644
index 0000000..f748b68
--- /dev/null
+++ b/web/pglister/lists/migrations/0058_list_subscriber_access.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.24 on 2023-04-03 10:41
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('lists', '0057_neverblockregexp'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='list',
+ name='subscriber_access',
+ field=models.BooleanField(default=False, help_text='Can subscribers get full access to the archives?', null=True),
+ ),
+ ]
diff --git a/web/pglister/lists/models.py b/web/pglister/lists/models.py
index dba9689..3df6252 100644
--- a/web/pglister/lists/models.py
+++ b/web/pglister/lists/models.py
@@ -163,6 +163,8 @@ class List(models.Model):
apikey_ro = models.CharField(max_length=100, null=False, blank=True, verbose_name="Read-only API key")
apikey_rw = models.CharField(max_length=100, null=False, blank=True, verbose_name="Read-write API key")
+ subscriber_access = models.BooleanField(null=True, blank=False, default=False,
+ help_text="Can subscribers get full access to the archives?")
def __str__(self):
return "{0}@{1}".format(self.name, self.domain.name)
--
2.40.0
[text/x-patch] 0001-pglister_sync-obtain-subscriber_access-from-API.patch (2.5K, 3-0001-pglister_sync-obtain-subscriber_access-from-API.patch)
download | inline diff:
From 6ea77326553f23fcfc3835e9211a8d777380d9d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 3 Apr 2023 11:24:29 +0200
Subject: [PATCH] pglister_sync: obtain subscriber_access from API
---
loader/pglister_sync.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/loader/pglister_sync.py b/loader/pglister_sync.py
index d7e1925..94ef5c7 100755
--- a/loader/pglister_sync.py
+++ b/loader/pglister_sync.py
@@ -60,20 +60,22 @@ if __name__ == "__main__":
'name': l['listname'],
})
if curs.rowcount == 0:
- curs.execute("INSERT INTO lists (listname, shortdesc, description, active, groupid, subscriber_access) SELECT %(name)s, %(name)s, %(desc)s, 't', groupid, 'f' FROM listgroups WHERE groupname=%(groupname)s RETURNING listid, listname", {
+ curs.execute("INSERT INTO lists (listname, shortdesc, description, active, groupid, subscriber_access) SELECT %(name)s, %(name)s, %(desc)s, 't', groupid, %(subscriber_access)s FROM listgroups WHERE groupname=%(groupname)s RETURNING listid, listname", {
'name': l['listname'],
'desc': l['longdesc'],
'groupname': l['group']['groupname'],
+ 'subscriber_access': l['subscriber_access'],
})
listid, name = curs.fetchone()
print("Added list %s" % name)
else:
listid, name = curs.fetchone()
- curs.execute("UPDATE lists SET shortdesc=%(name)s, description=%(desc)s, groupid=(SELECT groupid FROM listgroups WHERE groupname=%(groupname)s), active=true WHERE listid=%(id)s AND NOT (active AND shortdesc=%(name)s AND description=%(desc)s AND groupid=(SELECT groupid FROM listgroups WHERE groupname=%(groupname)s)) RETURNING listname", {
+ curs.execute("UPDATE lists SET shortdesc=%(name)s, description=%(desc)s, groupid=(SELECT groupid FROM listgroups WHERE groupname=%(groupname)s), active=true, subscriber_access=%(subscriber_access)s WHERE listid=%(id)s AND NOT (active AND shortdesc=%(name)s AND description=%(desc)s AND groupid=(SELECT groupid FROM listgroups WHERE groupname=%(groupname)s)) RETURNING listname", {
'id': listid,
'name': l['listname'],
'desc': l['longdesc'],
'groupname': l['group']['groupname'],
+ 'subscriber_access': l['subscriber_access'],
})
for n, in curs.fetchall():
print("Updated list %s " % n)
--
2.40.0
[text/x-patch] 0002-Add-subscriber_access-to-archives-API.patch (2.8K, 4-0002-Add-subscriber_access-to-archives-API.patch)
download | inline diff:
From e941294dfe3e82459efd2b36ca57c6ea807b814a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9lestin=20Matte?= <[email protected]>
Date: Mon, 3 Apr 2023 17:08:22 +0200
Subject: [PATCH 2/2] Add subscriber_access to archives API
---
web/pglister/lists/views_api.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/web/pglister/lists/views_api.py b/web/pglister/lists/views_api.py
index 24e41df..e957c2d 100644
--- a/web/pglister/lists/views_api.py
+++ b/web/pglister/lists/views_api.py
@@ -140,11 +140,11 @@ class ArchivesApi(View):
def get_lists(self, with_subscribers):
curs = connection.cursor()
if with_subscribers:
- curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname) AS group, COALESCE(array_agg(u.username) FILTER (WHERE u.username IS NOT NULL), ARRAY[]::text[]) AS subscribers FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id LEFT JOIN mailinglist_subscribers s ON s.listid=l.id LEFT JOIN auth_user u ON u.id=s.userid WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
+ curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname) AS group, COALESCE(array_agg(u.username) FILTER (WHERE u.username IS NOT NULL), ARRAY[]::text[]) AS subscribers, l.subscriber_access AS subscriber_access FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id LEFT JOIN mailinglist_subscribers s ON s.listid=l.id LEFT JOIN auth_user u ON u.id=s.userid WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
'archiveid': self.archiveserver.id,
})
else:
- curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname) AS group FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
+ curs.execute("SELECT l.id AS listid, l.name AS listname, d.name AS domain, l.shortdesc, l.longdesc, json_build_object('id', g.id, 'groupname', g.groupname) AS group, l.subscriber_access AS subscriber_access FROM lists_list l INNER JOIN lists_domain d ON l.domain_id=d.id INNER JOIN lists_listgroup g ON l.group_id=g.id WHERE l.archivedat_id=%(archiveid)s GROUP BY l.id, d.id, g.id ORDER BY 2,1", {
'archiveid': self.archiveserver.id,
})
columns = [col[0] for col in curs.description]
--
2.40.0
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-30 12:20 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
2022-02-02 08:16 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-02-05 17:24 ` Re: [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Magnus Hagander <[email protected]>
@ 2024-02-12 20:47 ` Célestin Matte <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Célestin Matte @ 2024-02-12 20:47 UTC (permalink / raw)
To: Magnus Hagander <[email protected]>; +Cc: PostgreSQL WWW <[email protected]>
On 05/02/2022 18:24, Magnus Hagander wrote:
> In general, I like that level of disconnect -- it should be possible
> to swap out the archive solution from underneath it, or indeed run the
> same pglister instance against multiple different types of archives.
Re-reading this thread, I now understand that the patches I sent later on did not satisfy this requirement.
Having no way to give access to without resorting to SQL does not seem ideal to me, though. Should I push a patch to activate the admin interface on pgarchives, as mentioned earlier in the thread?
--
Célestin Matte
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2024-02-12 20:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 17:37 [PATCH] pgarchives: pglister_sync: import lists with subscriber_access set to True Célestin Matte <[email protected]>
2022-01-28 17:43 ` Célestin Matte <[email protected]>
2022-01-30 12:20 ` Magnus Hagander <[email protected]>
2022-02-02 08:16 ` Célestin Matte <[email protected]>
2022-02-05 17:24 ` Magnus Hagander <[email protected]>
2023-02-28 10:39 ` Célestin Matte <[email protected]>
2023-04-03 15:16 ` Célestin Matte <[email protected]>
2024-02-12 20:47 ` Célestin Matte <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox