Code

Opened 18 months ago

Closed 9 months 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: master
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.

Attachments (0)

Change History (9)

comment:1 Changed 18 months ago by manelclos@…

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 17 months ago by claudep

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to 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 Changed 17 months ago by claudep

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

comment:4 Changed 14 months ago by claudep

  • Needs tests unset
  • Patch needs improvement unset

comment:5 Changed 14 months ago by gcc

  • 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 Changed 14 months ago by claudep

  • Patch needs improvement unset

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

comment:7 Changed 9 months ago by claudep

  • Version changed from 1.4 to master

Pull request just updated.

comment:8 Changed 9 months ago by timo

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 9 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.