Anyone have ideas on #16550 - custom SQL before/after syncdb?

400 views
Skip to first unread message

Jacob Kaplan-Moss

unread,
May 15, 2013, 9:01:11 PM5/15/13
to django-developers
Hi folks --

Does anyone have some clever thoughts on how to solve #16650?

https://code.djangoproject.com/ticket/16550#comment:7 is a good
summary of the problem: if you're using extensions, you need a way to
run some custom SQL in tests after the DB gets created by the test
harness but before syncdb runs.

The current patch and suggested solution on the ticket won't work (see
the thread), but I think it's a legit need and I'd like to come up
with a good solution and possibly reopen the ticket.

Jacob

Donald Stufft

unread,
May 15, 2013, 9:07:35 PM5/15/13
to django-d...@googlegroups.com
> --
> You received this message because you are subscribed to the Google Groups "Django developers" 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.com.
> Visit this group at http://groups.google.com/group/django-developers?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

pre_syncdb signal? syncdb is still called in tests right?

-----------------
Donald Stufft
PGP: 0x6E3CBCE93372DCFA // 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA

signature.asc

Daryl

unread,
May 15, 2013, 10:29:20 PM5/15/13
to django-d...@googlegroups.com
Jacob, going back a couple of levels, if its true that;
- sqlite creates the db for you, and the testdb is created for you
- sqlite could be considered to set permissions for you (ie set the owner of the db file)
- mysql does *not* create the db, but the testdb *is* created for you
- mysql requires you to issue a "grant all priv on..." on the db, but not the testdb
- i have no idea about postgresql or oracle.
and ...
- the various db backends know how to craft db specific DDL
then...
- would it be logical for the db backend to create the db for you *if* it doesn't already exist.

If the above are true, then why can't the same code that creates the testdb also create the db if it doesn't exist, and that would be the logical place to hook any post-create-db code such as user-defined data types etc?


This would/could also nullify some questions that new users have when they get tangled up at the syncdb point of the tutorial if they have not granted their usercode (from settings) access to the db. 

D






 

Danilo Bargen

unread,
May 16, 2013, 4:20:16 AM5/16/13
to django-d...@googlegroups.com
As a sidenote, there was a discussion about this on this mailing list a few months ago:

Anssi Kääriäinen

unread,
May 18, 2013, 1:03:35 PM5/18/13
to Django developers
On 16 touko, 11:20, Danilo Bargen <gez...@gmail.com> wrote:
> As a sidenote, there was a discussion about this on this mailing list a few
> months ago:
>
> https://groups.google.com/forum/#!searchin/django-developers/16550/dj...

I think a pre_sync signal is best approach. The signal should be
called either just after connecting to the (test) DB in syncdb
command, or maybe just after existing tables have been introspected so
that you know what tables are already there. Latter might be better as
syncdb can be ran multiple times, and you only need to CREATE
EXTENSION on initial sync. OTOH if you add one more model with
JSONField it seems you would need to run another CREATE EXTENSION, or
to investigate if some existing model has already ran CREATE
EXTENSION. So, defensive coding (that is, CREATE EXTENSION IF NOT
EXISTS) would be needed.

Another problem is that post_syncdb is called from flush command, too.
This seems wrong. Could we just add post_flush signal and send that
instead? Another option is to add a "for_flush" kwarg to the signal
parameters, but it feels just so wrong to have post_syncdb signal with
an argument that tells syncdb didn't happen after all. The reason for
post_syncdb from flush is creation of ContentTypes and Permissions
after truncation of those tables.

While the pre_syncdb signal approach has many shortcomings (how to
include the output in sqlall?), I think it is enough to solve this
problem for now. You can run CREATE EXTENSION IF NOT EXISTS and that
seems already a big step forward. Also, distinguishing post_flush from
post_syncdb seems like a good idea, but should be done in separate
ticket.

Later on migrations framework could offer a much better solution to
this. But pre_syncdb signal seems small enough to include already in
1.6. Maybe this could be done in the sprints?

- Anssi

Karol Sikora

unread,
May 18, 2013, 1:06:50 PM5/18/13
to django-d...@googlegroups.com

I can try to implement approach with pre_syncdb signal tomorrow. I think that is quite enough solution before deeper diggling into new migrations framework.

Karol

Donald Stufft

unread,
May 18, 2013, 1:15:22 PM5/18/13
to django-d...@googlegroups.com
There's already a patch on the ticket tracker for a pre_syncdb signal, and yesterday I started updating it and modifying it a bit as I needed this signal.

signature.asc

Andrew Godwin

unread,
May 20, 2013, 3:20:32 AM5/20/13
to django-d...@googlegroups.com
Of course, the long-term solution for this is probably migrations. The post_syncdb signal already causes me problems - as there's no good definition for it with migrations around (you basically have to send it right at the end for every model you think you touched).

However, the patch Donald linked would be a lot easier to emulate, so I'm not that against it.

Andrew
Message has been deleted

Donald Stufft

unread,
May 21, 2013, 6:51:09 AM5/21/13
to django-d...@googlegroups.com, django-d...@googlegroups.com
I run migrations in test. How else will you know your db reflects reality :/

On May 20, 2013, at 10:58 AM, charettes <chare...@gmail.com> wrote:

This makes me wonder if you're planing to introduce a `SOUTH_TEST_MIGRATE` setting analog when moving migration handling to core.

I think most people with a huge south migration history will set this setting to `False` to speedup testsuite execution and thus they couldn't be used for database setup.

Maybe people should just lean toward rebasing their migrations instead?

Shai Berger

unread,
May 21, 2013, 11:29:49 AM5/21/13
to django-d...@googlegroups.com
On Tuesday 21 May 2013, Donald Stufft wrote:
> I run migrations in test. How else will you know your db reflects reality
> :/
>

When you have a few hundred migrations, that's something you're willing to do
in your CI server, but not on your development machine.

Shai.

peter

unread,
May 21, 2013, 12:11:31 PM5/21/13
to django-d...@googlegroups.com
+1 on pre_syncdb

Donald Stufft

unread,
May 22, 2013, 11:53:46 PM5/22/13
to django-d...@googlegroups.com
--
You received this message because you are subscribed to the Google Groups "Django developers" 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.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

So I just submitted a PR for the pre_syncdb signal: https://github.com/django/django/pull/1200

After I get an eye or two on it, assuming no one has any issues with it I'll merge it.
signature.asc

Anssi Kääriäinen

unread,
May 23, 2013, 1:10:33 AM5/23/13
to Django developers
On 23 touko, 06:53, Donald Stufft <don...@stufft.io> wrote:
> So I just submitted a PR for the pre_syncdb signal:https://github.com/django/django/pull/1200
>
> After I get an eye or two on it, assuming no one has any issues with it I'll merge it.

Minor error in docs spotted, otherwise looks good to me.

I am thinking of adding a "for_action" kwarg for post_syncdb.
Currently there is no way to distinguish between flush and syncdb.
This makes it impossible to use the signal for schema changes. I guess
the kwarg could be useful for migrations, too.

Does this sound reasonable? Would the for_action kwarg help with
South?

The pre_syncdb signal isn't sent for flush. I think this is correct,
as I can't see anything you could do in pre_syncdb signal for flush.
If there are use cases, then I think adding for_action to pre_syncdb
should be done. IMO we can do these minor changes after alpha if need
be.

- Anssi
Reply all
Reply to author
Forward
0 new messages