Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python manage.py distill-local returns exit status 0 on failure #9

Closed
jayvdb opened this issue Dec 4, 2017 · 4 comments
Closed

python manage.py distill-local returns exit status 0 on failure #9

jayvdb opened this issue Dec 4, 2017 · 4 comments

Comments

@jayvdb
Copy link

jayvdb commented Dec 4, 2017

I am still investigating this, but as you are doing releases fairly quickly, maybe you're also interested in this problem.

coala/community#21

@meeb
Copy link
Owner

meeb commented Dec 4, 2017

Fixed in 1.1.

I had noticed this previously but no-one had requested it as a feature to date. As hacking this into the internals of Django would be quite laborious (general "exit >= 1" on any error in commands code) I've implemented this as a sys.exit(1) on any view rendering errors with the upstream trace preserved. This should suffice for your CI needs without bothering anyone else.

@meeb meeb closed this as completed Dec 4, 2017
@jayvdb
Copy link
Author

jayvdb commented Dec 4, 2017

I am surprised that it also occurs in Django 1.x ; it should be stable enough that implementation details like this 'just work'.
We also encountered it in Django 2.0 , so there is definitely a need to fix this upstream.
Thanks a heap for your tool and for rapid responses.

@jayvdb
Copy link
Author

jayvdb commented Dec 4, 2017

Poking around, I see https://code.djangoproject.com/ticket/24384 ; while it is not about rippling exceptions that go all the way up, the proposed solution there is to raise a specific exception CommandError. I'd be very surprised if using that exception was the 'proper' way to get a non-zero exit status, but I'm already surprised about the behaviour of manage.py so maybe that is it.

@meeb
Copy link
Owner

meeb commented Dec 4, 2017

Yes, this appears to have changed a bit in the Django command code since I last checked around the 1.6 era. I've just pushed django-distill 1.2 (4 releases in a day, woo...) which properly wraps the exceptions and should work (lightly tested) in Python2 and Python3. There is likely a deeper bug somewhere where non-manually-wrapped exceptions raised do not return a proper status code on exit. At the very least, I can remove the manual sys.exit in django-distill and let the command runner handle it which is a bit cleaner.

CommandError errors will just show the error message and exit with a status code of 1. To get the actually useful full trace you'll want to also add --traceback to your ./manage.py command.

This appears to work well in some test projects. Let me know if this solves it for you.

Note 1.2 also includes some previously shelved patches for better encoding support and MEDIA_URL files, but this shouldn't impact you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants