Storage does not follow Django docs recommendations

[c0dearm]

Hi!

First of all, thanks for your package. It looks great! However, I don't understand the reason why MinioBackend requires a bucket name as argument to instantiate. This, for instance, makes it impossible to set up the storage as the default storage for either statics or media files.

The Django docs are quite explicit about this: https://docs.djangoproject.com/en/3.2/howto/custom-file-storage/, a storage should not have init arguments.

This basically causes two issues:

1. For media files, you need to specify the storage on every file field you use
2. As far as I know it is impossible to use MinIO to store the statics collected by collectstatic

django-minio-storage solved this by allowing to set two settings: MINIO_STORAGE_MEDIA_BUCKET_NAME and MINIO_STORAGE_STATICS_BUCKET_NAME, so the respective buckets are used accordingly.

I wonder if it would be a good idea to do something similar for this project.

[toabi]

I have the same "problem". I now need to store static files in a minio bucket. We are already using this for FileFields and it works great.

A setting like MINIO_STATIC_BUCKET would be awesome.

[theriverman]

Thanks for the feedbacks to both of you! I do understand your described problem, and I will take a look at this issue in the coming days for sure.

[theriverman]

I've pushed 4447a3c166d1811e3db60e140c56ceb991521c3c to a feature branch and I've released a beta version of the package to PyPI under version v3.0.0b0.

Please get it manually from pip and let me know of your experiences (@c0dearm, @toabi):

pip install django-minio-backend==3.0.0b0

I've added two new sections to README.md, make sure you've updated your settings.py accordingly:

• Static Files Support
• Default File Storage Support

[toabi]

Cool, I'll give it a try and commend my findings here.

I think it's not the best idea that users now have to set MINIO_STATIC_FILES_BUCKET and MINIO_MEDIA_FILES_BUCKET to a valid bucket. (Otherwise django-admin initialize_buckets will fail).

Wouldn't it be possible to fail later when trying to access the static/media storage and they are not set?

Another issue I face: I'm using SASS + Compressor in a project so I tried to set this up as documented: https://django-compressor.readthedocs.io/en/stable/remote-storages/#using-staticfiles

But when I run collectstatic I get an error in https://github.com/theriverman/django-minio-backend/blob/v3.0.0b0/django_minio_backend/models.py#L267-L273

Traceback (most recent call last):
  File "/usr/bin/django-admin", line 8, in <module>
    sys.exit(execute_from_command_line())
  File "/usr/lib/python3.8/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python3.8/site-packages/django/core/management/__init__.py", line 395, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/lib/python3.8/site-packages/django/core/management/base.py", line 330, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/lib/python3.8/site-packages/django/core/management/base.py", line 371, in execute
    output = self.handle(*args, **options)
  File "/usr/lib/python3.8/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 194, in handle
    collected = self.collect()
  File "/usr/lib/python3.8/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 118, in collect
    handler(path, prefixed_path, storage)
  File "/usr/lib/python3.8/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 345, in copy_file
    if not self.delete_file(path, prefixed_path, source_storage):
  File "/usr/lib/python3.8/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 258, in delete_file
    target_last_modified = self.storage.get_modified_time(prefixed_path)
  File "/usr/lib/python3.8/site-packages/django_minio_backend/models.py", line 273, in get_modified_time
    return datetime.fromtimestamp(mktime(obj.last_modified))
TypeError: Tuple or struct_time argument required

Looks like obj and last_modified have those values:

<minio.datatypes.Object object at 0x7f08fb585a30>
2021-07-19 06:13:03+00:00 (type: <class 'datetime.datetime'>)

Changing the code to return obj.last_modified fixes this issue, although collectstatic now runs forever (edit: just noticed that we use fontawesome_free which contains millions of small files… which is not very good for non-local storage). For files which are not processed by compressor it seems to work though. I'll have to look at it later again.

[toabi]

I think this now works as expected (with the one change). The issue lies more on the side of my complex scss/compressor setup.

[theriverman]

Thanks a lot for your quick feedback!

I think it's not the best idea that users now have to set MINIO_STATIC_FILES_BUCKET and MINIO_MEDIA_FILES_BUCKET to a valid bucket. (Otherwise django-admin initialize_buckets will fail).

Wouldn't it be possible to fail later when trying to access the static/media storage and they are not set?

I think it's best to fail as soon as possible. Buckets should be created and configured (e.g.: policy settings) as early as possible to reduce bucket creation and configuration overhead during actual file operations.

However, I agree with you it's not very convenient to declare these buckets when someone would be happy with the default settings. So, I've made a little improvement regarding MINIO_STATIC_FILES_BUCKET and MINIO_MEDIA_FILES_BUCKET. Now, if they are not set at all, django-minio-backend will fall back to the default bucket names which are auto-generated-bucket-static-files and auto-generated-bucket-media-files respectively. I've also added some special treatment for this scenario around here: https://github.com/theriverman/django-minio-backend/blob/ce60623a55a068621b6fe4a578dbf2630bd50205/django_minio_backend/models.py#L60-L62 and here: https://github.com/theriverman/django-minio-backend/blob/ce60623a55a068621b6fe4a578dbf2630bd50205/django_minio_backend/models.py#L109-L111

This way initialize_buckets will not fail for them.

Regarding the issue with get_modified_time your correction is fine. I think the MinIO Python SDK behaviour has changed without me noticing. I've implemented your recommended solution.

I've pushed another beta package:

pip install django-minio-backend==3.0.0b2

[c0dearm]

Hi @theriverman,

Thank you a lot for the changes, it works like a charm!

I also agree with you any possible misconfiguration should fail as soon as possible, it really helps to catch issues in production before somebody starts complaining.

[theriverman]

Thanks for the confirmation, @c0dearm ! Is it okay for you too, @toabi ?

[toabi]

I think it works. I just couldn't make it run with compressor.

[theriverman]

Thanks. Let me know if there's anything I could do on my side to make it work.

[theriverman]

v3.0.0 has been released on PyPI

[toabi]

One year later, I tried it again together with compressor and remote storage. Sadly it's still not working.

The problem is different though. The error is now:

django.core.management.base.CommandError: An error occurred during rendering error/403.html: The joined path (/nl4nl-public/theme.scss) is located outside of the base path component (/usr/lib/python3.9/site-packages/admin_interface/static)

One suspicion is that it maybe has something to do with this part, and that this backend maybe doesn't return the basename as filename and therefore the find() gets triggered? I have no idea.

Ah, there's already a ticket there with the same issue I have: https://github.com/django-compressor/django-compressor/issues/1119

It looks like if the backend would return the name with the https://miniohost/… it might work. Not sure where to test this.

[theriverman]

@toabi Could you give me the full trace, please? I'm not familiar with django-compressor and I don't understand what's wrong. django-minio-backend (in theory) follows the recommended Django storage implementation. If it wouldn't, then it couldn't be used as a default media/static storage, but that's not the case - it works well.

If there's any backward compatible tweak I could add to django-minio-backend, then I'm happy to do that but it's not clear to me what's the problem here.

• What function is called by compressor?
• What is the expected result?
• What is the actual result?

[toabi]

I got so annoyed by the situation that I removed compressor (which we only used for sass compiling) from the project. It had more pros than cons. So I'm now really happy with this module.

But maybe @eugenewere wants to follow up on this.