Upgrade to latest django-storages and add support for Azure Blob Storage #3794
Conversation
| @@ -591,7 +591,7 @@ def absolute_mp3_path(self): | |||
| content = self.get_mp3_content() | |||
| kobocat_storage.save(self.mp3_storage_path, ContentFile(content)) | |||
|
|
|||
| if not isinstance(kobocat_storage, KobocatS3Boto3Storage): | |||
| if isinstance(kobocat_storage, KobocatFileSystemStorage): | |||
There was a problem hiding this comment.
I reversed this to make anything not local storage, use the S3 like storage. Which maybe works for azure too. I really hate this but it's the only quick way I could think to make it work. It should really not be so S3 specific and just happen to maybe work in other places.
| DEFAULT_FILE_STORAGE = os.environ.get('KPI_DEFAULT_FILE_STORAGE') | ||
| if DEFAULT_FILE_STORAGE == 'storages.backends.s3boto3.S3Boto3Storage': | ||
| # Force usage of custom S3 tellable Storage | ||
| DEFAULT_FILE_STORAGE = 'kobo.apps.storage_backends.s3boto3.S3Boto3Storage' |
There was a problem hiding this comment.
👏
I was going to say: we need to open an issue in kobo-install to support the new class.
There was a problem hiding this comment.
Yeah I'd rather no one know we do this horrible hack 😄 and we may some day be able to remove it. So we don't want under users changing it.
| bucket = django_settings.KOBOCAT_AWS_STORAGE_BUCKET_NAME | ||
| super().__init__(acl=None, bucket=bucket, **settings) | ||
| super().__init__(*args, **kwargs) |
There was a problem hiding this comment.
🤔 bucket should be enforced to django_settings.KOBOCAT_AWS_STORAGE_BUCKET_NAME, right?
Am I missing something or it's not passed to parent anymore?
Should not be something like this instead?:
def __init__(self, *args, **kwargs):
kwargs['bucket_name'] = django_settings.KOBOCAT_AWS_STORAGE_BUCKET_NAME
super().__init__(*args, **kwargs)BTW, it should be bucket_name now, bucket is deprecated. I did not notice it when I created the class. I have changed it in the Django 3.2 branch.
There was a problem hiding this comment.
Updated, I tested by making an alternative bucket name for kc
|
|
||
| class KobocatAzureStorage(AzureStorage): | ||
| def __init__(self, *args, **kwargs): | ||
| azure_container = django_settings.KOBOCAT_AZURE_CONTAINER |
There was a problem hiding this comment.
🤔 Same thoughts as above? How does KobocatAzureStorage know when to write since azure_container is not passed to the parent constructor?
Should not be something like this instead?:
def __init__(self, *args, **kwargs):
kwargs['azure_container'] = django_settings. KOBOCAT_AZURE_CONTAINER
super().__init__(*args, **kwargs)There was a problem hiding this comment.
Removed entirely as discussed. We won't support an alternative kc azure container.
Remove support for having seperate kobocat azure container for kobocat
jnm
changed the title
jnm
changed the title
Description
Upgrade django-storages package. Adds support for Azure Blob Storage, outside of django-private-storage
Checklist
This PR does not address django-private-storage continuing to use only S3. Therefor this PR only partially support Azure.
Related to kobotoolbox/kobocat#816