-
Notifications
You must be signed in to change notification settings - Fork 68
Conversation
removed long_description for heroku python buildpack support
@@ -120,7 +132,7 @@ def handle_noargs(self, **options): | |||
clear_display = 'This will overwrite existing files!' | |||
|
|||
if self.interactive: | |||
confirm = raw_input(u""" | |||
confirm = input(u""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is copied straight from Django itself so I'd rather maintain it being as similar as possible to Django. I am not sure what the differences between raw_input and input are though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The big difference is raw_input doesn't exist in Python 3.x. Which Django module did you pull this code from? I'd be interested in seeing how they handle this issue for Py2/3 compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to update the code so that it reflects the latest stable version of Django, I'd be fine with that. Alternatively, if you'd be able to modify the output in a cleaner way without duplicating all the code, that'd be preferable ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you say modify the output, you mean try to find a way to override the output message in the function provided by Django 1.6+'s collectstatic?
It looks like they import 'input' from the included six utility in Django 1.6. If I can't figure out the DRYest way of doing it, I'll just remove the logic up top that looks for the built-in and use the six module instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's what I meant.
Cool, just make sure it's compatible with 1.4.x as well 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason that you're declaring this function anyway? I don't see anything collectfast specific happening in handle_noargs. Why override at all?
edit: I removed handle_noargs entirely from a local branch and collectstatic still behaves as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To output the number of skipped files: https://github.com/antonagestam/collectfast/blob/master/collectfast/management/commands/collectstatic.py#L142
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I missed that line in the template. I can't think of a better way at this point to override only the template and summary due to the order of execution. For now, I'll wrap testing on Django 1.4+ and Python 2/3 and push the changes for the input call still overriding handle_noargs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing done. All is well in my environment with Python 2/3 and Django 1.4 - 1.7 using S3 and boto with collectfast.
long_description was originally commented out due to incompatibility with the Python buildpack in Heroku and Dokku. Uncommented for wide distribution as the compatibility issue isn't the fault of this code, but the pip version (1.3) in the buildpack itself.
… Imports Python 2/3 compatible input from django.utils.six
@archen Thank you for taking your time with this ticket! |
@antonagestam absolutely! hopefully someone else finds it as useful as I did. Thanks for putting the original package together! |
Added some logic for Python 3 support that maintains Python 2 support