Rename CRITERIA_ASSETS_* to EXTERNAL_ASSETS_*#320
Merged
ArthurCRodrigues merged 1 commit intoMay 16, 2026
Conversation
Replace CRITERIA_ASSETS_* environment variables with EXTERNAL_ASSETS_* across the codebase and docs. Updated .env.example, docker-compose.yml, autograder/services/assets/resolver.py (in-memory cache env var), autograder/services/assets/s3_provider.py (bucket name env var), and docs/features/setup_config_feature.md. Make sure to update deployment and local environment variables to the new names to avoid configuration breakage.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR renames the external asset injection environment variables from the legacy CRITERIA_ASSETS_* names to EXTERNAL_ASSETS_* across the autograder runtime configuration and documentation, aligning terminology with the “external assets” feature.
Changes:
- Renamed S3 bucket env var to
EXTERNAL_ASSETS_BUCKET_NAMEin code,docker-compose.yml,.env.example, and docs. - Renamed in-memory asset cache limit env var to
EXTERNAL_ASSETS_IN_MEMORY_CACHE_LIMITin the asset resolver and.env.example. - Updated setup-config feature documentation to reflect the new naming.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
docs/features/setup_config_feature.md |
Updates documented env var name(s) for S3-backed asset injection. |
docker-compose.yml |
Updates the dev compose environment variable name for the assets bucket. |
autograder/services/assets/s3_provider.py |
Switches bucket name lookup to EXTERNAL_ASSETS_BUCKET_NAME. |
autograder/services/assets/resolver.py |
Switches cache limit lookup to EXTERNAL_ASSETS_IN_MEMORY_CACHE_LIMIT. |
.env.example |
Updates example env var names to the new EXTERNAL_ASSETS_* scheme. |
Comments suppressed due to low confidence (1)
autograder/services/assets/s3_provider.py:20
S3AssetProvidernow only readsEXTERNAL_ASSETS_BUCKET_NAME. This makes existing deployments that still setCRITERIA_ASSETS_BUCKET_NAMEfail at runtime (bucket_name becomesNone), and the resulting error will be a generic S3 fetch failure. Consider reading the new env var first but falling back to the old name with a deprecation warning, and/or explicitly validatingbucket_nameduring initialization to raise a clear configuration error mentioning the expected env var(s).
# Environment variables
self.bucket_name = os.getenv("EXTERNAL_ASSETS_BUCKET_NAME")
self.access_key = os.getenv("AWS_ACCESS_KEY_ID") or os.getenv("AWS_ACCESS_ID")
self.secret_key = os.getenv("AWS_SECRET_ACCESS_KEY")
self.region = os.getenv("AWS_REGION", "us-east-1")
self.endpoint_url = os.getenv("S3_ENDPOINT_URL")
Comment on lines
12
to
15
| def __init__(self): | ||
| in_memory_limit = int(os.getenv("CRITERIA_ASSETS_IN_MEMORY_CACHE_LIMIT", "100")) | ||
| in_memory_limit = int(os.getenv("EXTERNAL_ASSETS_IN_MEMORY_CACHE_LIMIT", "100")) | ||
| self.cache_manager = AssetCacheManager(in_memory_limit=in_memory_limit) | ||
| self.provider = S3AssetProvider(self.cache_manager) |
Comment on lines
12
to
15
| def __init__(self): | ||
| in_memory_limit = int(os.getenv("CRITERIA_ASSETS_IN_MEMORY_CACHE_LIMIT", "100")) | ||
| in_memory_limit = int(os.getenv("EXTERNAL_ASSETS_IN_MEMORY_CACHE_LIMIT", "100")) | ||
| self.cache_manager = AssetCacheManager(in_memory_limit=in_memory_limit) | ||
| self.provider = S3AssetProvider(self.cache_manager) |
Comment on lines
70
to
71
| - `AWS_ACCESS_ID` & `AWS_SECRET_ACCESS_KEY`: Credentials for the S3 provider. | ||
|
|
ArthurCRodrigues
approved these changes
May 16, 2026
ArthurCRodrigues
left a comment
ArthurCRodrigues
left a comment
Member
There was a problem hiding this comment.
I'll just fix our server's vars and this will be merged
ArthurCRodrigues
deleted the
301-rename-criteria_assets-to-external_assets
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Solution
Replace CRITERIA_ASSETS_* environment variables with EXTERNAL_ASSETS_* across the codebase and docs. Updated .env.example, docker-compose.yml, autograder/services/assets/resolver.py (in-memory cache env var), autograder/services/assets/s3_provider.py (bucket name env var), and docs/features/setup_config_feature.md.
Make sure to update deployment and local environment variables to the new names to avoid configuration breakage.
Further clarifications
Related issues
#301
Checklist