Skip to content

Conversation

@jawad-khan
Copy link
Member

@jawad-khan jawad-khan commented Dec 5, 2025

Remove DEFAULT_FILE_STORAGE and STATICFILES_STORAGES. This addresses issue.

Tested with this yml.

SECRET_KEY: dummy secret key
FILE_STORAGE_BACKEND:
  AWS_S3_OBJECT_PARAMETERS:
     CacheControl: max-age=31536000
  AWS_QUERYSTRING_AUTH: false
  AWS_QUERYSTRING_EXPIRE: false
  AWS_S3_CUSTOM_DOMAIN: stage.edx-cdn.org
  AWS_STORAGE_BUCKET_NAME: stage-edx
  DEFAULT_FILE_STORAGE: storages.backends.s3boto3.S3Boto3Storage
  MEDIA_ROOT: media
  MEDIA_URL: https://stage.edx-cdn.org/media/
STORAGES:
  default:
    BACKEND: storages.backends.s3boto3.S3Boto3Storage
  staticfiles:
    BACKEND: storages.backends.s3boto3.S3Boto3Storage

Screenshot 2025-12-05 at 11 54 53 PM

Run JavaScript tests locally with Karma

There is work being done on a fix to get Karma to run in CI. Until then, however, contributors are required to run these tests locally.

  • Make sure you are inside the devstack container
  • Run make test-karma
  • All tests pass

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes deprecated Django storage configuration settings (DEFAULT_FILE_STORAGE and STATICFILES_STORAGE) from both production and devstack environments. The codebase now relies on Django's modern STORAGES dictionary configuration (defined in base.py), which is the standard approach since Django 4.2 and required in Django 5.x (the project uses Django 5.2.7).

  • Removes code that attempted to override STORAGES dictionary values using deprecated environment variables and YAML configuration keys
  • Updates inline documentation to reflect the removal of deprecated settings handling

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
credentials/settings/production.py Removes YAML-based configuration for deprecated DEFAULT_FILE_STORAGE and STATICFILES_STORAGE settings and their mapping to the STORAGES dictionary
credentials/settings/devstack.py Removes environment variable-based configuration for deprecated DEFAULT_FILE_STORAGE and STATICFILES_STORAGE settings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


# Load the files storage backend settings for django storages
# In django==4.2.24 following line sets the DEFAULT_FILE_STORAGE and other AWS variables as per YAML.
# In django==4.2.24 following line sets AWS variables as per YAML.
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment references django==4.2.24, but requirements/django.txt indicates the project is using Django 5.2.7. The comment should be updated to reflect the actual Django version being used.

Suggested change
# In django==4.2.24 following line sets AWS variables as per YAML.
# In django==5.2.7 the following line sets AWS variables as per YAML.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the inline comment.

@awais786 awais786 self-requested a review December 9, 2025 11:32
vars().update(config_from_yaml)

FILE_STORAGE_BACKEND = config_from_yaml.get("FILE_STORAGE_BACKEND", {})
default_backend = FILE_STORAGE_BACKEND.pop("DEFAULT_FILE_STORAGE", None)
Copy link
Contributor

@awais786 awais786 Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without STORAGES dict inside prod yaml we cant make these changes. It will break production

@awais786 awais786 self-requested a review December 9, 2025 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants