-
Notifications
You must be signed in to change notification settings - Fork 69
Conversation
|
||
def is_boto3(storage): | ||
return type(storage).__name__ == 'S3Boto3Storage' | ||
return has_boto and isinstance(storage, S3Boto3Storage) |
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.
would this work if we didn't have boto/boto3 installed? (how do we test that)
edit: thinking through it, it should work, but I'm thinking we should cover that test case.
tested it locally, this works even if you don't have boto/boto3.
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.
Yeah, the isinstance(storage, S3Boto3Storage)
will only get evaluated if has_boto
is True
, which will only happen if the import is successful. Would be good to test this but I think it might be a lot of effort.
collectfast/tests/test_command.py
Outdated
@test | ||
@override_django_settings( | ||
STATICFILES_STORAGE="django.contrib.staticfiles.storage.StaticFilesStorage") | ||
@override_setting("enabled", True) |
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.
did you mean for this to be False? (though I would say we also need to test this particular config)
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.
@pachewise Good find! Thanks for pointing that out! :)
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.
(And that config is already tested in test_basics
btw)
fixes #120