Add support for S3 regions #962
Conversation
✅ Deploy Preview for antenna-preview canceled.
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for S3 regions to enable users in different AWS regions (particularly Europe) to properly configure their S3 storage backends. Previously, the region parameter was omitted for simplicity as university clouds didn't require it and the default region worked for existing AWS users.
- Adds optional
regionfield to theS3StorageSourcemodel and corresponding configuration - Updates S3 client initialization to properly handle region configuration
- Fixes existing type annotation issues in S3 utility functions
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 |
|---|---|
| ami/main/models.py | Adds nullable region field to S3StorageSource model and includes it in the config property |
| ami/main/migrations/0075_s3storagesource_region.py | Database migration to add the region field to existing S3 storage sources |
| ami/utils/s3.py | Updates S3 client/session initialization to use region and fixes type annotations |
| ami/tests/fixtures/storage.py | Updates test fixture to include region parameter |
| config/settings/base.py | Adds S3_TEST_REGION environment variable configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| import boto3 | ||
| import boto3.resources.base | ||
| import boto3.session |
There was a problem hiding this comment.
The import boto3.session is not used in this file. The code uses boto3.Session which is available through the main boto3 import. This import should be removed.
| import boto3.session |
| config=boto_config, | ||
| ) | ||
|
|
||
| client = typing.cast(S3Client, client) |
There was a problem hiding this comment.
This type cast is unnecessary. The session.client() method already returns the correct type and the variable is already properly typed. Remove this line to simplify the code.
| client = typing.cast(S3Client, client) |
| endpoint_url=config.endpoint_url, | ||
| # api_version="s3v4", | ||
| ) | ||
| s3 = typing.cast(S3ServiceResource, s3) |
There was a problem hiding this comment.
This type cast is unnecessary. The session.resource() method already returns the correct type. Remove this line to simplify the code.
| s3 = typing.cast(S3ServiceResource, s3) |
Summary
Previously we omitted the
regionparameter for simplicity because the S3 implementations in the university compute clouds do not use regions and the default region worked for existing AWS S3 users. Now we have some users in Europe that need to specify the S3 region in order for the storage backend to sync, to display source images in the UI, and to processes source images.List of Changes
regionfield to theS3StorageSourcemodelHow to Test the Changes
.
Screenshots
If applicable, add screenshots to help explain this PR (ex. Before and after for UI changes).
Deployment Notes
.
Checklist