fix exit logic in checkdata management command #204
Conversation
Pull Request Test Coverage Report for Build 18685632626Details
💛 - Coveralls |
| self.eprint(f'\nTotal check time: {timedelta(seconds=duration)}') | ||
| sys.exit(1 if has_desync else 0) | ||
| if has_desync: | ||
| raise CommandError('Desync data found.', returncode=1) |
There was a problem hiding this comment.
How does that CommandError behave in call_command? Will it raise there or just return the non 0 returncode? (Not sure atm, if call_command uses a subprocess...)
There was a problem hiding this comment.
Sorry I missed your question. I think call_command does nothing fancy so it will just bubble up as documented here:
CommandError(returncode=1)Exception class indicating a problem while executing a management command.
If this exception is raised during the execution of a management command from a command line console, it will be caught and turned into a nicely-printed error message to the appropriate output stream (i.e., stderr); as a result, raising this exception (with a sensible description of the error) is the preferred way to indicate that something has gone wrong in the execution of a command. It accepts the optional returncode argument to customize the exit status for the management command to exit with, using sys.exit().
If a management command is called from code through call_command(), it’s up to you to catch the exception when needed.
It's true that for a command that explicitly does a check, throwing an exception is a bit counter intuitive, as nothing has really gone wrong... but it's inherent of management commands being a CLI. We could factor the logic out of the management command and expose it in the python API, where it could return a boolean. Would you prefer that ?
|
Btw - just saw that the command output is also not properly done, it uses print instead of |
3 participants
fix for #202