Hooverdan96 commented 3 months ago

I took the plunge to update my system to 5.0.9.0 and created a backup configuration file prior to the upgrade from 4.x.

Unfortunately, when trying to upload the file from a local windows (win 11) drive, a major error message occurred, and the file was not uploaded.

To ensure it's not related to the version difference, I created a backup file of the current config (only had the pools imported and the initial setup, including the Rockon service mapped to the appropriate share and turned on, no additional users created nor Rockons).

When trying to upload that file it unfortunately also failed. In both cases the following trace was issued (in a pop-up) but no file was uploaded. So I am not even to the config restore step at this time.

[16/Jun/2024 11:48:05] ERROR [storageadmin.util:45] Exception: Could not find config for 'default' in settings.STORAGES.
Traceback (most recent call last):
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/core/files/storage/handler.py", line 35, in __getitem__
    return self._storages[alias]
           ~~~~~~~~~~~~~~^^^^^^^
KeyError: 'default'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/core/files/storage/handler.py", line 38, in __getitem__
    params = self.backends[alias]
             ~~~~~~~~~~~~~^^^^^^^
KeyError: 'default'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception
    yield
  File "/opt/rockstor/src/rockstor/storageadmin/views/config_backup.py", line 734, in post
    cbo = ConfigBackup.objects.create(filename=filename, config_backup=file_obj)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/query.py", line 658, in create
    obj.save(force_insert=True, using=self.db)
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 814, in save
    self.save_base(
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 877, in save_base
    updated = self._save_table(
              ^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 1020, in _save_table
    results = self._do_insert(
              ^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 1061, in _do_insert
    return manager._insert(
           ^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/query.py", line 1805, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1821, in execute_sql
    for sql, params in self.as_sql():
                       ^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1745, in as_sql
    value_rows = [
                 ^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1746, in <listcomp>
    [
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1747, in <listcomp>
    self.prepare_value(field, self.pre_save_val(field, obj))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1695, in pre_save_val
    return field.pre_save(obj, add=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/fields/files.py", line 317, in pre_save
    file.save(file.name, file.file, save=False)
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/fields/files.py", line 92, in save
    name = self.field.generate_filename(self.instance, name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/db/models/fields/files.py", line 337, in generate_filename
    return self.storage.generate_filename(filename)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/utils/functional.py", line 266, in inner
    self._setup()
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/core/files/storage/__init__.py", line 42, in _setup
    self._wrapped = storages[DEFAULT_STORAGE_ALIAS]
                    ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/rockstor/.venv/lib/python3.11/site-packages/django/core/files/storage/handler.py", line 40, in __getitem__
    raise InvalidStorageError(
django.core.files.storage.handler.InvalidStorageError: Could not find config for 'default' in settings.STORAGES.

@FroggyFlox any idea what this could be? I renamed the file to a very short name (no time stamp or underscores, etc.) but that didn't make a difference.

Hooverdan96 commented 3 months ago

It seems, that Django wants to have a default entry in the STORAGES section of the settings file?

Based on this: https://stackoverflow.com/questions/76411302/could-not-find-config-for-static-files-in-settings-storages

I just added the line here:

https://github.com/rockstor/rockstor-core/blob/5cdc84ae2b50f79496cbf9591e1baf6a2026ff59/src/rockstor/settings.py#L206-L212

So it looks like this:

# STATICFILES_STORAGE = "pipeline.storage.PipelineManifestStorage"
STORAGES = {
    "default": {
        "BACKEND": "django.core.files.storage.FileSystemStorage",
    },
    "staticfiles": {
        "BACKEND": "pipeline.storage.PipelineManifestStorage",
    },
}

Restarted Rockstor, and the upload worked. The restore is now in progress (let's hope that works without a hitch). This insert might not be the right expression to use, though. If it is indeed correct, I can create a pull request for it, as this seems quite important that the config restore works if people migrate and install a newer version rather than an in place upgrade ...

FroggyFlox commented 3 months ago

I was going to reply when you had already found the solution and tested it.

Briefly, this seems to be a depredation from 4.2 (not sure how we didn't get warnings about that, though... unless we missed them):

https://docs.djangoproject.com/en/5.0/releases/4.2/#custom-file-storages

I would need to read more to know what the best solution is but yours seems ok at first glance.

Hooverdan96 commented 3 months ago

I haven't checked, but I assume there is no unit test for a file upload that would highlight this error?

phillxnet commented 3 months ago

@Hooverdan96 & @FroggyFlox From what looks like the canonical reference: https://docs.djangoproject.com/en/5.0/releases/4.2/#custom-file-storages

We have, regarding the STORAGE config element that we added as part of our Django updates:

... It also controls storage engines for managing files (the "default" key ...

That looks to be the bit I missed when adding the new STORAGE bit. And it seems it only shows up when actually used, such as in @Hooverdan96 reproducer. The depecation message I reacted to concerned only the staticfiles key for this newer STORAGE config element, and no longer appeared once I'd transitioned our prior (depricated) static files config.

I'll reproduce locally and prepare a pull request with the assumed missing default entry to double the found fix (hopefully). If all is well we can roll new rpms and have this fix out soon under say a 5.0.10-0 versioned rpm.

phillxnet commented 3 months ago

Managing files references:

This document describes Django’s file access APIs for files such as those uploaded by a user.

As initially expected we should have a default already, according to:

Django’s default file storage is 'django.core.files.storage.FileSystemStorage'. If you don’t explicitly provide a storage system in the default key of the STORAGES setting, this is the one that will be used.

So it seems we have triggered a corner case of sorts with our current config and there is then no default key as per the above quote; and @Hooverdan96 report re:

Could not find config for 'default' in settings.STORAGES

@FroggyFlox Given the above, and from admittedly only a quick read, @Hooverdan96 finding is the way to go here.

I.e. we rely on a default that is some-how no in effect: even given the above "If you don’t explicitly provide a storage system in the default key of the STORAGES setting, this is the one that will be used.". Alas it's a show-shopper, so we need a work-around: and if defining the default to it's default looks to be harmless enough. And in @Hooverdan96 findings: a fix :).

phillxnet commented 3 months ago

Closing as: Fixed by #2850

rockstor / rockstor-core

5.0.6-0 to 5.0.9-0 Configuration Backup file upload fails #2846

Managing files references: