Opened 17 years ago
Closed 13 years ago
#7043 closed Bug (fixed)
Bad verbosity in loaddata command when fixtures location passed by param baddly
Reported by: | Manuel Saelices | Owned by: | Manuel Saelices |
---|---|---|---|
Component: | Core (Management commands) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Look, for example this execution:
~$ ./manage.py loaddata something_wrong_but_developer_doesnt_know.xml ~$ <-- no error message
But in [source:django/trunk/django/core/management/commands/loaddata.py loaddata.py] there is this code fragment:
if object_count == 0: if verbosity >= 2: print "No fixtures found."
I think maybe it could be better to remove the verbosity condition.
Attachments (1)
Change History (10)
by , 17 years ago
Attachment: | loaddata.diff added |
---|
comment:1 by , 17 years ago
Status: | new → assigned |
---|
follow-up: 3 comment:2 by , 17 years ago
comment:3 by , 17 years ago
Replying to russellm:
I'm not neccessarily disagreeing, but why do you think the verbosity should be removed? There is a reasonable argument for having the message the way it is - with normal verbosity, you get output only if a fixture is loaded, so it's easy to integrate with a shell script to determine if data was actually loaded. What is your reasoning for making the 'no fixtures' error compulsory?
I think will be useful for developers. Myself was trying to loaddata, but docs not explain if fixtures location is a path to an xml, or if is an application name or whatever. The problem is that you put an application name, and you don't know if fixtures are successfully loaded. Others commands, like dumpdata, fails if application is not found.
comment:4 by , 16 years ago
+1 on this, since it just bit me again (I've used fixtures before, I just forgot to give it a .json
extension). It's very surprising not to get an error.
If a script wants to check if data was actually loaded, that's what return values ($?
) are for.
comment:5 by , 16 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:6 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:7 by , 13 years ago
Easy pickings: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
This is a real problem. If anything, the accepted pattern for command-line utilities is to be silent on success, noisy on failure (see, for example, rules 11 and 12 at http://www.faqs.org/docs/artu/ch01s06.html ). Silence on failure by default is bad.
comment:8 by , 13 years ago
Actually, many things have changed since this was reported four years ago.
Particularly, r13612 introduced a check that aborts the execution of the loaddata command when the no objects are found in al least one of the fixtures. This means the if fixture_object_count == 0:
check (such is the name of the variable now, it contains the number of all objects found in all fixtures) is never reached.
The fix would be then to simply remove the dead code that performs this check.
I'm not neccessarily disagreeing, but why do you think the verbosity should be removed? There is a reasonable argument for having the message the way it is - with normal verbosity, you get output only if a fixture is loaded, so it's easy to integrate with a shell script to determine if data was actually loaded. What is your reasoning for making the 'no fixtures' error compulsory?