Обсуждение: [PATCH] pgarchives: Bugfix: allow message's parentid to be null
Hello, Upon execution of load_messages.py, when creating a new thread, I get a crash: (Sorry about the format, I get this out of strace) Traceback (most recent call last):\n File \"/srv/pgarchives/local/loader/load_message.py\", lin\ e 174, in <module>\n ap.store(conn, listid, opt.overwrite, opt.overwrite)\n File \"/srv/pgarchives/local/loader/li\ b/storage.py\", line 232, in store\n 'rawtxt': bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\ e in column \"parentid\" violates not-null constraint\nDETAIL: Failing row contains (2, 2, cmatte <cmatte@[my_domain]\ .tld>, helloworld@[my_pglister_domain], , Test archive after redeployment 2, 2021-10-25 14:13:32+00, 1635171212.3\ 49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f, null, \\x46726f6d2068656c6c6f776f726c642d6f776\ e65722b617263686976654062..., '2':5A 'after':3A 'archive':2A 'hello':6 'redeployment':4A 'test...).\n\n I think the way to fix this is simply to allow message.parentid to be null. Cheers, -- Célestin Matte
Вложения
Attached: associated migration On 25/10/2021 16:52, Célestin Matte wrote: > Hello, > > Upon execution of load_messages.py, when creating a new thread, I get a crash: > (Sorry about the format, I get this out of strace) > > Traceback (most recent call last):\n File \"/srv/pgarchives/local/loader/load_message.py\", lin\ > e 174, in <module>\n ap.store(conn, listid, opt.overwrite, opt.overwrite)\n File \"/srv/pgarchives/local/loader/li\ > b/storage.py\", line 232, in store\n 'rawtxt': bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\ > e in column \"parentid\" violates not-null constraint\nDETAIL: Failing row contains (2, 2, cmatte <cmatte@[my_domain]\ > .tld>, helloworld@[my_pglister_domain], , Test archive after redeployment 2, 2021-10-25 14:13:32+00, 1635171212.3\ > 49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f, null, \\x46726f6d2068656c6c6f776f726c642d6f776\ > e65722b617263686976654062..., '2':5A 'after':3A 'archive':2A 'hello':6 'redeployment':4A 'test...).\n\n > > I think the way to fix this is simply to allow message.parentid to be null. > > Cheers, > -- Célestin Matte
Вложения
On Tue, Oct 26, 2021 at 8:31 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
Attached: associated migration
On 25/10/2021 16:52, Célestin Matte wrote:
> Hello,
>
> Upon execution of load_messages.py, when creating a new thread, I get a crash:
> (Sorry about the format, I get this out of strace)
>
> Traceback (most recent call last):\n File \"/srv/pgarchives/local/loader/load_message.py\", lin\
> e 174, in <module>\n ap.store(conn, listid, opt.overwrite, opt.overwrite)\n File \"/srv/pgarchives/local/loader/li\
> b/storage.py\", line 232, in store\n 'rawtxt': bytearray(self.rawtxt),\npsycopg2.errors.NotNullViolation: null valu\
> e in column \"parentid\" violates not-null constraint\nDETAIL: Failing row contains (2, 2, cmatte <cmatte@[my_domain]\
> .tld>, helloworld@[my_pglister_domain], , Test archive after redeployment 2, 2021-10-25 14:13:32+00, 1635171212.3\
> 49997.1068459.nullmailer@[my_domain], hello world\n\n\n\n, null, f, null, \\x46726f6d2068656c6c6f776f726c642d6f776\
> e65722b617263686976654062..., '2':5A 'after':3A 'archive':2A 'hello':6 'redeployment':4A 'test...).\n\n
>
> I think the way to fix this is simply to allow message.parentid to be null.
Yeah, this looks to be another disreptency between the Django model and the schema.sql file. The table when created from schema.sql will allow nulls. So I can confirm that the field *should* allow nulls, and the only reason it hasn't been an actual problem is that we never ever do any writes to the system through the django model.
That said, I think the correct solutoin is still, as I mentioned in one of the other threads, to completely move everything into the initial migration, instead of doing a piecemeal hack that's going to result in a lot of small migrations for no real reason. We should just accept that the existing one is broken and replace it.
If you're willing to look into working on that I'd be happy to review on that one, but if it seems a bit too much I can work on that one myself as well, but it might take a bit before I have time to dig into it properly.
//Magnus
> > That said, I think the correct solutoin is still, as I mentioned in one of the other threads, to completely move everythinginto the initial migration, instead of doing a piecemeal hack that's going to result in a lot of small migrationsfor no real reason. We should just accept that the existing one is broken and replace it. Can everything be moved to django's model though? The schema.sql file defines several other components (indexes, text searchconfigurations, text search dictionaries, functions, triggers) and I'm not sure django is able to handle all of thatby itself. Is it? Or do you only want to move tables? Cheers, -- Célestin Matte
On Wed, Oct 27, 2021 at 12:04 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
>
> That said, I think the correct solutoin is still, as I mentioned in one of the other threads, to completely move everything into the initial migration, instead of doing a piecemeal hack that's going to result in a lot of small migrations for no real reason. We should just accept that the existing one is broken and replace it.
Can everything be moved to django's model though? The schema.sql file defines several other components (indexes, text search configurations, text search dictionaries, functions, triggers) and I'm not sure django is able to handle all of that by itself. Is it? Or do you only want to move tables?
The models can't handle them, but migrations can.
I'm thinking similar to pglister, where we use django models to create the parts that *can* be created, and then RunSQL() migrations to do the rest. That way the pure SQL parts can still run as part of the same flow, and can be mixed in with the django generated ones. The end result in the database should be the same, but avoiding to have to maintain a separate .sql file for it to keep in sync.