Opened 6 years ago

Closed 3 years ago

#14300 closed Bug (fixed)

manage.py sqlcustom fails if models are in a submodule

Reported by: David Rogers Owned by: Fede Heinz
Component: Core (Management commands) Version: 1.2
Severity: Normal Keywords:
Cc: emyller@…, timograham@…, vlastimil@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is similar to #6961, from what I've seen. According to the docs for 1.2:

Django provides a hook for passing the database arbitrary SQL that's executed just after the CREATE TABLE 
statements when you run syncdb. You can use this hook to populate default records, or you could also create
SQL functions, views, triggers, etc.

The hook is simple: Django just looks for a file called sql/<modelname>.sql, in your app directory, where 
<modelname> is the model's name in lowercase.

This doesn't work when the "models" module is a directory instead of a file, i.e. project/application/models/ vice project/application/models.py. Apparently, sqlcustom is using similar logic to what was described in #6961, since I can move my sql/ directory into the models/ directory - a la project/application/models/sql/ - and everything works.

Attachments (1)

customsql_model_package.patch (5.2 KB) - added by Fede Heinz 6 years ago.
Fixes the bug, adds tests

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by Fede Heinz

Owner: changed from nobody to Fede Heinz

Changed 6 years ago by Fede Heinz

Fixes the bug, adds tests

comment:2 Changed 6 years ago by Fede Heinz

Has patch: set
Resolution: fixed
Status: newclosed
Triage Stage: UnreviewedAccepted

The attached patch fixes the bug, and adds tests to check that both cases (models module is a directory or a file) work.

comment:3 Changed 6 years ago by Ramiro Morales

Resolution: fixed
Status: closedreopened

Reopening. The fixed status is set when a commit lands in the code repository.

comment:4 Changed 6 years ago by Julien Phalip

Patch needs improvement: set
Severity: Normal
Type: Bug

Unittests are now preferred over doctests in the django codebase. Could you rewrite your tests accordingly? Thanks!

comment:5 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 Changed 5 years ago by Evandro Myller

Cc: emyller@… added

comment:8 Changed 4 years ago by Claude Paroz

#19338 and #19358 are duplicates.

comment:9 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:10 Changed 3 years ago by Tim Graham

Cc: timograham@… added
Patch needs improvement: unset

comment:11 Changed 3 years ago by Tim Graham

Also reported in #15519 the PR there addresses backwards incompatibility by searching both app/sql and app/models/sql with a DeprecationWarning if the latter is used. I'm not a huge fan of adding extra code and tests to support that for a release or two, but I'll do it if necessary.

comment:12 in reply to:  11 Changed 3 years ago by Vlastimil Zíma

Cc: vlastimil@… added

Replying to timo:

Also reported in #15519 the PR there addresses backwards incompatibility by searching both app/sql and app/models/sql with a DeprecationWarning if the latter is used. I'm not a huge fan of adding extra code and tests to support that for a release or two, but I'll do it if necessary.

In ticket:15519#comment:2 jacob required to use both locations. My patch also contains a test. The patch does not apply, because of all the changes in tests, but it should be easy to fix.

Edit: Fix the comment link.

Last edited 3 years ago by Vlastimil Zíma (previous) (diff)

comment:13 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 31c13a99bb9ebdaf12ccab4e880c5da930d86e79:

Fixed #14300 -- Fixed initial SQL location if models is a package.

Thanks al_the_x for the report and fheinz for the draft patch.

Note: See TracTickets for help on using tickets.
Back to Top