Opened 12 years ago

Closed 11 years ago

#19657 closed Bug (fixed)

sql, sqlcustom, sqlindexes and sqlall do not take allow_syncdb into account

Reported by: manelclos@… Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: sql sqlall database router
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When synchronizing the database, sql for models will appear in all databases, this is, not respecting allow_syncdb returning False.

Change History (9)

comment:1 by manelclos@…, 12 years ago

Has patch: set

Patch at ​https://github.com/django/django/pull/667

comment:2 by Claude Paroz, 12 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Bug

I don't think you need to add a new parameter to the sql_* functions, as you should be able to get the db alias through connection.alias (to be confirmed).

I noticed that the sql_* were missing tests. This might need a new regressiontests/sql_commands test directory.
​http://ci.djangoproject.com/job/Django%20Coverage/HTML_Coverage_Report/_var_lib_jenkins_jobs_Django%20Coverage_workspace_django_core_management_commands_sql.html

comment:3 by Claude Paroz, 12 years ago

I've just added some base tests for those commands: [718afcafc202940cd15799459178a2534b8217d5]

comment:4 by Claude Paroz, 12 years ago

Needs tests: unset
Patch needs improvement: unset

New pull request in ​https://github.com/django/django/pull/1216

comment:5 by Chris Wilson, 12 years ago

Patch needs improvement: set

Please could you add tests that the correct value of include_auto_created is passed for each set of SQL statements, and that it's honoured correctly by the filtered_app_models function?

Actually, would it make more sense for filtered_app_models to be a method of Router, so that it's easier to override?

comment:6 by Claude Paroz, 11 years ago

Patch needs improvement: unset

I've just added a second commit to the PR to address your latest concern.

comment:7 by Claude Paroz, 11 years ago

Version: 1.4 → master

Pull request just updated.

comment:8 by Tim Graham, 11 years ago

Triage Stage: Accepted → Ready for checkin

comment:9 by Claude Paroz <claude@…>, 11 years ago

Resolution: → fixed
Status: new → closed

In 2992f428614349b0dfe0f4183905f492fd3f62c2:

Fixed #19657 -- Made sql commands honor allow_migrate

Thanks Manel Clos for the report and the initial patch, and
Marc Tamlyn and Tim Graham for the review.

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