File Service Config Environments & Provider Types #166
Conversation
tmthecoder
commented
Feb 12, 2025
tmthecoder
commented
- Adds support for environments in the file service config
- Adds support for provider types (needed for blob storage support)
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Disclaimer: This comment was AI-generated and is not necessarily completely accurate. Please take code comments with a grain of salt.
PR Summary
This PR adds environment support and provider type functionality to the file service configuration, enabling multi-environment configurations and support for different storage providers like S3 and Azure.
- Added required
configEnvfield to file operations and composite keys in database schema, with default value 'test' for new configurations - Introduced
FileProviderTypeenum (S3, AZURE) with conversion utilities between RPC and Prisma types - Missing test coverage for Azure provider type and environment-specific edge cases across multiple services
- Inconsistent error messages and validation in file bucket operations (particularly in
file_bucket.controller.ts) - Hardcoded region 'us-east-005' in
file_bucket.service.tsshould be configurable
Score: 75/100
Backend Checklist Issues:
- Input validation needs improvement for environment and provider type fields
- Edge cases for different environments not fully tested
- Error handling could be more specific for environment-related issues
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
42 file(s) reviewed, 22 comment(s)
Edit PR Review Bot Settings | Greptile
packages/api-gateway/src/modules/file_download/file_download.controller.ts
Show resolved
Hide resolved
| const fileProviderBody = { | ||
| providerName: '', | ||
| publicAccessKey: 'Test Public Access Key', | ||
| privateAccessKey: 'Test Private Access Key', | ||
| baseUrl: 'https://aws.amazon.com/s3', | ||
| type: 'S3', | ||
| }; |
There was a problem hiding this comment.
logic: Request body structure is inconsistent with the valid test case (lines 218-225) which uses nested accessKey object
| const fileProviderBody = { | |
| providerName: '', | |
| publicAccessKey: 'Test Public Access Key', | |
| privateAccessKey: 'Test Private Access Key', | |
| baseUrl: 'https://aws.amazon.com/s3', | |
| type: 'S3', | |
| }; | |
| const fileProviderBody = { | |
| providerName: '', | |
| accessKey: { | |
| publicAccessKey: 'Test Public Access Key', | |
| privateAccessKey: 'Test Private Access Key', | |
| }, | |
| baseUrl: 'https://aws.amazon.com/s3', | |
| type: 'S3', | |
| }; |
There was a problem hiding this comment.
Review was wrong (it should be what you put) but it does seem like we have the wrong access key format in several of the e2e tests (nested)--could you take a look?
| @IsNotEmpty() | ||
| @Transform(toEnum) | ||
| @ApiProperty({ description: 'File provider type' }) | ||
| type: FileProviderProto.ProviderType; |
There was a problem hiding this comment.
style: Consider adding example values to @ApiProperty to show valid provider types (S3, AZURE)
