Skip to content

Commit

Permalink
Replace DEFAULT_FILE_STORAGE with STORAGES in codebase
Browse files Browse the repository at this point in the history
Django deprecated the use of DEFAULT_FILE_STORAGES and STATICFILES_STORAGE
in v4.2 and removed it in v5.0.

It provides a compat layer in 4.2, so even if the user set this value
using the legacy key, the value will be available in django.conf.settings
as:

*  settings.STORAGES['default']['BACKEND']  # default storage
*  settings.STORAGES['staticfiles']['BACKEND']  # staticfiles storage

This commit updates the codebase to use the new value.
  • Loading branch information
pedro-psb committed Dec 18, 2024
1 parent 61120d4 commit b6fe356
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 39 deletions.
9 changes: 9 additions & 0 deletions CHANGES/plugin_api/5404.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Started using `STORAGES` internally instead of `DEFAULT_FILE_STORAGE` and `STATICFILES_STORAGE`,
which was deprecated in Django 4.2.

For compatibility, plugins must replace access to:

* `settings.DEFAULT_FILE_STORAGE` with `settings.STORAGES["default"]["BACKEND"]`
* `settings.STATICFILES_STORAGE` with `settings.STORAGES["staticfiles"]["BACKEND"]`

See the new storage structure in [Django docs](https://docs.djangoproject.com/en/4.2/ref/settings/#std-setting-STORAGES).
8 changes: 4 additions & 4 deletions pulp_file/tests/functional/api/test_download_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

from pulpcore.tests.functional.utils import get_files_in_manifest, download_file

from pulpcore.app import settings
from pulpcore.client.pulp_file import FileFilePublication, RepositorySyncURL


Expand Down Expand Up @@ -54,7 +53,8 @@ def test_download_policy(
download_policy,
):
"""Test that "on_demand" and "streamed" download policies work as expected."""
if download_policy == "on_demand" and "SFTP" in pulp_settings.DEFAULT_FILE_STORAGE:
settings = pulp_settings
if download_policy == "on_demand" and "SFTP" in settings.STORAGES["default"]["BACKEND"]:
pytest.skip("This storage technology is not properly supported.")

remote = file_remote_ssl_factory(
Expand Down Expand Up @@ -148,7 +148,7 @@ def test_download_policy(
assert expected_checksum == actual_checksum
if (
download_policy == "immediate"
and settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem"
and settings.STORAGES["default"]["BACKEND"] != "pulpcore.app.models.storage.FileSystem"
and settings.REDIRECT_TO_OBJECT_STORAGE
):
content_disposition = downloaded_file.response_obj.headers.get("Content-Disposition")
Expand Down Expand Up @@ -187,7 +187,7 @@ def test_download_policy(
content_unit = expected_files_list[4]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
# The S3 test API project doesn't handle invalid Range values correctly
if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem":
with pytest.raises(ClientResponseError) as exc:
range_header = {"Range": "bytes=-1-11"}
download_file(content_unit_url, headers=range_header)
Expand Down
6 changes: 3 additions & 3 deletions pulpcore/app/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,11 @@ def _ensure_default_domain(sender, **kwargs):
if (
settings.HIDE_GUARDED_DISTRIBUTIONS != default.hide_guarded_distributions
or settings.REDIRECT_TO_OBJECT_STORAGE != default.redirect_to_object_storage
or settings.DEFAULT_FILE_STORAGE != default.storage_class
or settings.STORAGES["default"]["BACKEND"] != default.storage_class
):
default.hide_guarded_distributions = settings.HIDE_GUARDED_DISTRIBUTIONS
default.redirect_to_object_storage = settings.REDIRECT_TO_OBJECT_STORAGE
default.storage_class = settings.DEFAULT_FILE_STORAGE
default.storage_class = settings.STORAGES["default"]["BACKEND"]
default.save(skip_hooks=True)


Expand Down Expand Up @@ -392,7 +392,7 @@ def _get_permission(perm):

def _populate_artifact_serving_distribution(sender, apps, verbosity, **kwargs):
if (
settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem"
settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem"
or not settings.REDIRECT_TO_OBJECT_STORAGE
):
try:
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/app/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def secret_key_check(app_configs, **kwargs):
def storage_paths(app_configs, **kwargs):
warnings = []

if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem":
try:
media_root_dev = Path(settings.MEDIA_ROOT).stat().st_dev
except OSError:
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/app/importexport.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def export_artifacts(export, artifact_pks):
with ProgressReport(**data) as pb:
pb.BATCH_INTERVAL = 5000

if settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] != "pulpcore.app.models.storage.FileSystem":
with tempfile.TemporaryDirectory(dir=".") as temp_dir:
for offset in range(0, len(artifact_pks), EXPORT_BATCH_SIZE):
batch = artifact_pks[offset : offset + EXPORT_BATCH_SIZE]
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/app/tasks/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def _export_to_file_system(path, relative_paths_to_artifacts, method=FS_EXPORT_M
ValidationError: When path is not in the ALLOWED_EXPORT_PATHS setting
"""
using_filesystem_storage = (
settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem"
settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem"
)

if method != FS_EXPORT_METHODS.WRITE and not using_filesystem_storage:
Expand Down
6 changes: 4 additions & 2 deletions pulpcore/app/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ def get_artifact_url(artifact, headers=None, http_method=None):
if settings.DOMAIN_ENABLED:
loc = f"domain {artifact_domain.name}.storage_class"
else:
loc = "settings.DEFAULT_FILE_STORAGE"
loc = "settings.STORAGES['default']['BACKEND']"

raise NotImplementedError(
f"The value {loc}={artifact_domain.storage_class} does not allow redirecting."
Expand Down Expand Up @@ -582,7 +582,9 @@ def get_default_domain():
try:
default_domain = Domain.objects.get(name="default")
except Domain.DoesNotExist:
default_domain = Domain(name="default", storage_class=settings.DEFAULT_FILE_STORAGE)
default_domain = Domain(
name="default", storage_class=settings.STORAGES["default"]["BACKEND"]
)
default_domain.save(skip_hooks=True)

return default_domain
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/pytest_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ def _settings_factory(storage_class=None, storage_settings=None):
"AZURE_CONNECTION_STRING",
]
settings = storage_settings or dict()
backend = storage_class or pulp_settings.DEFAULT_FILE_STORAGE
backend = storage_class or pulp_settings.STORAGES["default"]["BACKEND"]
for key in keys[backend]:
if key not in settings:
settings[key] = getattr(pulp_settings, key, None)
Expand Down
7 changes: 3 additions & 4 deletions pulpcore/tests/functional/api/test_artifact_distribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
import subprocess
from hashlib import sha256

from django.conf import settings


OBJECT_STORAGES = (
"storages.backends.s3boto3.S3Boto3Storage",
Expand All @@ -12,7 +10,8 @@
)


def test_artifact_distribution(random_artifact):
def test_artifact_distribution(random_artifact, pulp_settings):
settings = pulp_settings
artifact_uuid = random_artifact.pulp_href.split("/")[-2]

commands = (
Expand All @@ -29,7 +28,7 @@ def test_artifact_distribution(random_artifact):
hasher = sha256()
hasher.update(response.content)
assert hasher.hexdigest() == random_artifact.sha256
if settings.DEFAULT_FILE_STORAGE in OBJECT_STORAGES:
if settings.STORAGES["default"]["BACKEND"] in OBJECT_STORAGES:
content_disposition = response.headers.get("Content-Disposition")
assert content_disposition is not None
filename = artifact_uuid
Expand Down
6 changes: 3 additions & 3 deletions pulpcore/tests/functional/api/test_crd_artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import uuid
import pytest

from django.conf import settings
from pulpcore.client.pulpcore import ApiException


Expand Down Expand Up @@ -145,10 +144,11 @@ def test_upload_mixed_attrs(pulpcore_bindings, pulpcore_random_file):


@pytest.mark.parallel
def test_delete_artifact(pulpcore_bindings, pulpcore_random_file, gen_user):
def test_delete_artifact(pulpcore_bindings, pulpcore_random_file, gen_user, pulp_settings):
"""Verify that the deletion of artifacts is prohibited for both regular users and
administrators."""
if settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem":
settings = pulp_settings
if settings.STORAGES["default"]["BACKEND"] != "pulpcore.app.models.storage.FileSystem":
pytest.skip("this test only works for filesystem storage")
media_root = settings.MEDIA_ROOT

Expand Down
14 changes: 9 additions & 5 deletions pulpcore/tests/functional/api/test_crud_domains.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import string
import json
from pulpcore.client.pulpcore import ApiException
from pulpcore.app import settings

from pulpcore.tests.functional.utils import PulpTaskError

Expand Down Expand Up @@ -51,15 +50,16 @@ def test_crud_domains(pulpcore_bindings, monitor_task):


@pytest.mark.parallel
def test_default_domain(pulpcore_bindings):
def test_default_domain(pulpcore_bindings, pulp_settings):
"""Test properties around the default domain."""
settings = pulp_settings
domains = pulpcore_bindings.DomainsApi.list(name="default")
assert domains.count == 1

# Read the default domain, ensure storage is set to default
default_domain = domains.results[0]
assert default_domain.name == "default"
assert default_domain.storage_class == settings.DEFAULT_FILE_STORAGE
assert default_domain.storage_class == settings.STORAGES["default"]["BACKEND"]
assert default_domain.redirect_to_object_storage == settings.REDIRECT_TO_OBJECT_STORAGE
assert default_domain.hide_guarded_distributions == settings.HIDE_GUARDED_DISTRIBUTIONS

Expand Down Expand Up @@ -92,8 +92,9 @@ def test_default_domain(pulpcore_bindings):


@pytest.mark.parallel
def test_active_domain_deletion(pulpcore_bindings, monitor_task):
def test_active_domain_deletion(pulpcore_bindings, monitor_task, pulp_settings):
"""Test trying to delete a domain that is in use, has objects in it."""
settings = pulp_settings
if not settings.DOMAIN_ENABLED:
pytest.skip("Domains not enabled")
name = str(uuid.uuid4())
Expand Down Expand Up @@ -133,8 +134,10 @@ def test_orphan_domain_deletion(
gen_object_with_cleanup,
monitor_task,
tmp_path,
pulp_settings,
):
"""Test trying to delete a domain that is in use, has objects in it."""
settings = pulp_settings
if not settings.DOMAIN_ENABLED:
pytest.skip("Domains not enabled")
body = {
Expand Down Expand Up @@ -177,8 +180,9 @@ def test_orphan_domain_deletion(


@pytest.mark.parallel
def test_special_domain_creation(pulpcore_bindings, gen_object_with_cleanup):
def test_special_domain_creation(pulpcore_bindings, gen_object_with_cleanup, pulp_settings):
"""Test many possible domain creation scenarios."""
settings = pulp_settings
if not settings.DOMAIN_ENABLED:
pytest.skip("Domains not enabled")
# This test needs to account for which environment it is running in
Expand Down
13 changes: 6 additions & 7 deletions pulpcore/tests/functional/api/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import pytest

from django.conf import settings
from jsonschema import validate
from pulpcore.client.pulpcore import ApiException

Expand Down Expand Up @@ -59,24 +58,24 @@


@pytest.mark.parallel
def test_get_authenticated(test_path, pulpcore_bindings):
def test_get_authenticated(test_path, pulpcore_bindings, pulp_settings):
"""GET the status path with valid credentials.
Verify the response with :meth:`verify_get_response`.
"""
response = pulpcore_bindings.StatusApi.status_read()
verify_get_response(response.to_dict(), STATUS)
verify_get_response(response.to_dict(), STATUS, pulp_settings)


@pytest.mark.parallel
def test_get_unauthenticated(test_path, pulpcore_bindings, anonymous_user):
def test_get_unauthenticated(test_path, pulpcore_bindings, anonymous_user, pulp_settings):
"""GET the status path with no credentials.
Verify the response with :meth:`verify_get_response`.
"""
with anonymous_user:
response = pulpcore_bindings.StatusApi.status_read()
verify_get_response(response.to_dict(), STATUS)
verify_get_response(response.to_dict(), STATUS, pulp_settings)


@pytest.mark.parallel
Expand Down Expand Up @@ -127,7 +126,7 @@ def test_storage_per_domain(
assert default_status.storage != domain_status.storage


def verify_get_response(status, expected_schema):
def verify_get_response(status, expected_schema, settings):
"""Verify the response to an HTTP GET call.
Verify that several attributes and have the correct type or value.
Expand All @@ -142,7 +141,7 @@ def verify_get_response(status, expected_schema):
assert status["content_settings"]["content_path_prefix"] is not None

assert status["storage"]["used"] is not None
if settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] != "pulpcore.app.models.storage.FileSystem":
assert status["storage"]["free"] is None
assert status["storage"]["total"] is None
else:
Expand Down
14 changes: 8 additions & 6 deletions pulpcore/tests/functional/api/using_plugin/test_orphans.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import os
import pytest

from pulpcore.app import settings


def test_content_orphan_filter(
file_bindings,
Expand Down Expand Up @@ -68,12 +66,14 @@ def test_orphans_delete(
random_artifact,
file_random_content_unit,
monitor_task,
pulp_settings,
):
settings = pulp_settings
# Verify that the system contains the orphan content unit and the orphan artifact.
content_unit = file_bindings.ContentFilesApi.read(file_random_content_unit.pulp_href)
artifact = pulpcore_bindings.ArtifactsApi.read(random_artifact.pulp_href)

if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem":
# Verify that the artifacts are on disk
relative_path = pulpcore_bindings.ArtifactsApi.read(content_unit.artifact).file
artifact_path1 = os.path.join(settings.MEDIA_ROOT, relative_path)
Expand All @@ -89,7 +89,7 @@ def test_orphans_delete(
with pytest.raises(file_bindings.ApiException) as exc:
file_bindings.ContentFilesApi.read(file_random_content_unit.pulp_href)
assert exc.value.status == 404
if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem":
assert os.path.exists(artifact_path1) is False
assert os.path.exists(artifact_path2) is False

Expand All @@ -100,15 +100,17 @@ def test_orphans_cleanup(
random_artifact,
file_random_content_unit,
monitor_task,
pulp_settings,
):
settings = pulp_settings
# Cleanup orphans with a nonzero orphan_protection_time
monitor_task(pulpcore_bindings.OrphansCleanupApi.cleanup({"orphan_protection_time": 10}).task)

# Verify that the system contains the orphan content unit and the orphan artifact.
content_unit = file_bindings.ContentFilesApi.read(file_random_content_unit.pulp_href)
artifact = pulpcore_bindings.ArtifactsApi.read(random_artifact.pulp_href)

if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem":
# Verify that the artifacts are on disk
relative_path = pulpcore_bindings.ArtifactsApi.read(content_unit.artifact).file
artifact_path1 = os.path.join(settings.MEDIA_ROOT, relative_path)
Expand All @@ -123,7 +125,7 @@ def test_orphans_cleanup(
with pytest.raises(file_bindings.ApiException) as exc:
file_bindings.ContentFilesApi.read(file_random_content_unit.pulp_href)
assert exc.value.status == 404
if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem":
assert os.path.exists(artifact_path1) is False
assert os.path.exists(artifact_path2) is False

Expand Down
2 changes: 1 addition & 1 deletion pulpcore/tests/unit/models/test_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def test_create_read_delete_content(tmp_path):

@pytest.mark.django_db
def test_storage_location(tmp_path, settings):
if settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem":
if settings.STORAGES["default"]["BACKEND"] != "pulpcore.app.models.storage.FileSystem":
pytest.skip("Skipping test for nonlocal storage.")

tf = tmp_path / "ab"
Expand Down

0 comments on commit b6fe356

Please sign in to comment.