Opened 6 years ago

Closed 4 years ago

#11481 closed Uncategorized (duplicate)

loaddata doesn't return the proper exit code on error

Reported by: justinlilly Owned by: justinlilly
Component: Core (Other) Version: master
Severity: Normal Keywords: loaddata commands
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When loaddata errors, it doesn't return a proper error code. Instead, it just returns.

Attachments (2)

loaddata_proper_err_code.diff (815 bytes) - added by justinlilly 6 years ago.
Patch which uses sys.exit(1) instead of return.
loaddata_patch_ver_2.diff (2.0 KB) - added by justinlilly 6 years ago.

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by justinlilly

Patch which uses sys.exit(1) instead of return.

comment:1 Changed 6 years ago by Mnewman

  • Keywords loaddata commands added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

This looks like a valid point. There are several other places in that command that should do the same thing, i.e. http://code.djangoproject.com/browser/django/trunk/django/core/management/commands/loaddata.py#L141, http://code.djangoproject.com/browser/django/trunk/django/core/management/commands/loaddata.py#L181

Why print the errors and not raise them in these cases, let python return the error code?

Changed 6 years ago by justinlilly

comment:2 Changed 6 years ago by justinlilly

  • Patch needs improvement unset

I think the reason for printing the error messages and not returning exceptions is that we want to run the rollbacks. This could be fixed with a finally statement, but requires mucking with the code structure more. This seems like the simplest things that works.

I've also added another patch which corrects the other places where an error is printed, but not returning the right status code.

comment:3 Changed 6 years ago by nailor

...or you could raise django.core.management.CommandError and let it raise through the whole stack. It would make the manage.py to exit with exit status 1.

comment:4 Changed 4 years ago by julien

  • Resolution set to duplicate
  • Severity set to Normal
  • Status changed from new to closed
  • Type set to Uncategorized

Dupe of #10200.

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