Fix storage directory permission issues
- Modified config.py to use environment variables for storage paths - Added better error handling for directory creation - Updated file_service.py to handle permission errors gracefully - Changed default storage path to './storage' instead of '/app/storage' - Added comprehensive documentation about storage configuration - Added logging for better debugging of file operations - Updated README with storage configuration guidelines
This commit is contained in:
parent
aaac87335a
commit
9e8f0a412f
36
README.md
36
README.md
@ -94,14 +94,40 @@ Interactive API documentation is available at:
|
||||
|
||||
The API uses environment variables for configuration:
|
||||
|
||||
| Variable | Description | Default Value |
|
||||
|------------------|----------------------------------------------|------------------------|
|
||||
| PROJECT_NAME | Name of the project | "File Upload Download API" |
|
||||
| MAX_FILE_SIZE | Maximum allowed file size in bytes | 10485760 (10MB) |
|
||||
| Variable | Description | Default Value |
|
||||
|----------------------|----------------------------------------------|------------------------|
|
||||
| PROJECT_NAME | Name of the project | "File Upload Download API" |
|
||||
| MAX_FILE_SIZE | Maximum allowed file size in bytes | 10485760 (10MB) |
|
||||
| STORAGE_BASE_DIR | Base directory for all storage | "./storage" |
|
||||
| FILE_STORAGE_DIR | Directory for storing uploaded files | "{STORAGE_BASE_DIR}/files" |
|
||||
| DB_DIR | Directory for the SQLite database | "{STORAGE_BASE_DIR}/db" |
|
||||
| SQLALCHEMY_DATABASE_URL | Full database connection URL | "sqlite:///{DB_DIR}/db.sqlite" |
|
||||
|
||||
## File Storage
|
||||
|
||||
Files are stored in the `/app/storage/files` directory. File metadata is stored in a SQLite database at `/app/storage/db/db.sqlite`.
|
||||
By default, files are stored in the `./storage/files` directory. File metadata is stored in a SQLite database at `./storage/db/db.sqlite`.
|
||||
|
||||
### Storage in Containerized Environments
|
||||
|
||||
When running in a containerized environment like Docker, you should handle storage paths carefully:
|
||||
|
||||
1. **Using Volume Mounts**: Mount a persistent volume to the storage directory:
|
||||
```bash
|
||||
docker run -v /host/path/to/storage:/app/storage your-image-name
|
||||
```
|
||||
|
||||
2. **Using Environment Variables**: Configure storage locations with environment variables:
|
||||
```bash
|
||||
docker run -e STORAGE_BASE_DIR=/data -v /host/path/to/data:/data your-image-name
|
||||
```
|
||||
|
||||
3. **Pre-creating Directories**: Ensure directories exist with proper permissions:
|
||||
```bash
|
||||
mkdir -p ./storage/files ./storage/db
|
||||
chmod 777 ./storage/files ./storage/db # For development only
|
||||
```
|
||||
|
||||
> **Note**: The application will continue running even if it cannot create storage directories, but file operations will fail. This allows the application to start and serve documentation even if storage is not properly configured.
|
||||
|
||||
## Development
|
||||
|
||||
|
@ -1,24 +1,34 @@
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
from pydantic_settings import BaseSettings, SettingsConfigDict
|
||||
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class Settings(BaseSettings):
|
||||
PROJECT_NAME: str = "File Upload Download API"
|
||||
API_V1_STR: str = "/api/v1"
|
||||
|
||||
# Base directory for storage
|
||||
STORAGE_DIR: Path = Path("/app/storage")
|
||||
# Base directory for storage - can be overridden by environment variable
|
||||
STORAGE_BASE_DIR: str = os.environ.get("STORAGE_BASE_DIR", "./storage")
|
||||
|
||||
# File storage directory
|
||||
FILE_STORAGE_DIR: Path = STORAGE_DIR / "files"
|
||||
# File storage directory - derived from base dir but can be overridden
|
||||
FILE_STORAGE_DIR: Optional[str] = os.environ.get("FILE_STORAGE_DIR")
|
||||
|
||||
# Database
|
||||
DB_DIR: Path = STORAGE_DIR / "db"
|
||||
SQLALCHEMY_DATABASE_URL: str = f"sqlite:///{DB_DIR}/db.sqlite"
|
||||
# Database directory - derived from base dir but can be overridden
|
||||
DB_DIR: Optional[str] = os.environ.get("DB_DIR")
|
||||
|
||||
# Database URL - derived from DB_DIR but can be overridden
|
||||
SQLALCHEMY_DATABASE_URL: Optional[str] = os.environ.get("SQLALCHEMY_DATABASE_URL")
|
||||
|
||||
# Configure max file size (10MB by default)
|
||||
MAX_FILE_SIZE: int = 10 * 1024 * 1024 # 10MB in bytes
|
||||
MAX_FILE_SIZE: int = int(
|
||||
os.environ.get("MAX_FILE_SIZE", 10 * 1024 * 1024)
|
||||
) # 10MB in bytes
|
||||
|
||||
model_config = SettingsConfigDict(case_sensitive=True, env_file=".env")
|
||||
|
||||
@ -26,6 +36,44 @@ class Settings(BaseSettings):
|
||||
# Create settings instance
|
||||
settings = Settings()
|
||||
|
||||
# Ensure storage directories exist
|
||||
settings.DB_DIR.mkdir(parents=True, exist_ok=True)
|
||||
settings.FILE_STORAGE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
# Resolve directory paths if not explicitly set via environment variables
|
||||
if not settings.FILE_STORAGE_DIR:
|
||||
settings.FILE_STORAGE_DIR = os.path.join(settings.STORAGE_BASE_DIR, "files")
|
||||
|
||||
if not settings.DB_DIR:
|
||||
settings.DB_DIR = os.path.join(settings.STORAGE_BASE_DIR, "db")
|
||||
|
||||
if not settings.SQLALCHEMY_DATABASE_URL:
|
||||
db_path = os.path.join(settings.DB_DIR, "db.sqlite")
|
||||
settings.SQLALCHEMY_DATABASE_URL = f"sqlite:///{db_path}"
|
||||
|
||||
# Convert string paths to Path objects for easier manipulation
|
||||
storage_base_path = Path(settings.STORAGE_BASE_DIR)
|
||||
db_path = Path(settings.DB_DIR)
|
||||
file_storage_path = Path(settings.FILE_STORAGE_DIR)
|
||||
|
||||
# Try to create directories with better error handling
|
||||
try:
|
||||
# Ensure base storage directory exists
|
||||
storage_base_path.mkdir(parents=True, exist_ok=True)
|
||||
logger.info(f"Base storage directory: {storage_base_path}")
|
||||
|
||||
# Ensure DB directory exists
|
||||
db_path.mkdir(parents=True, exist_ok=True)
|
||||
logger.info(f"Database directory: {db_path}")
|
||||
|
||||
# Ensure file storage directory exists
|
||||
file_storage_path.mkdir(parents=True, exist_ok=True)
|
||||
logger.info(f"File storage directory: {file_storage_path}")
|
||||
except PermissionError as e:
|
||||
logger.warning(
|
||||
f"Permission denied when creating directories: {e}. "
|
||||
f"The application may need to be run with appropriate permissions or "
|
||||
f"you should pre-create these directories with correct permissions."
|
||||
)
|
||||
# We'll continue running, as the application might be able to function
|
||||
# if the directories already exist with correct permissions
|
||||
except Exception as e:
|
||||
logger.error(f"Error creating storage directories: {e}")
|
||||
# We'll still continue running in case the application can operate
|
||||
# without direct file access (e.g., if using external storage)
|
||||
|
@ -1,18 +1,21 @@
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
from fastapi import UploadFile, HTTPException, status
|
||||
from pathlib import Path
|
||||
from typing import BinaryIO
|
||||
from typing import BinaryIO, Tuple
|
||||
|
||||
from app.core.config import settings
|
||||
from app.models.file import File
|
||||
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class FileService:
|
||||
"""Service for handling file operations."""
|
||||
|
||||
@staticmethod
|
||||
async def save_file(upload_file: UploadFile) -> tuple[str, str, int]:
|
||||
async def save_file(upload_file: UploadFile) -> Tuple[str, str, int]:
|
||||
"""
|
||||
Save an uploaded file to disk.
|
||||
|
||||
@ -25,33 +28,48 @@ class FileService:
|
||||
Raises:
|
||||
HTTPException: If file size exceeds the maximum allowed size
|
||||
"""
|
||||
# Generate a unique filename to prevent collisions
|
||||
original_filename = upload_file.filename or "unnamed_file"
|
||||
unique_filename = File.generate_unique_filename(original_filename)
|
||||
try:
|
||||
# Generate a unique filename to prevent collisions
|
||||
original_filename = upload_file.filename or "unnamed_file"
|
||||
unique_filename = File.generate_unique_filename(original_filename)
|
||||
|
||||
# Define the path where the file will be saved
|
||||
file_path = settings.FILE_STORAGE_DIR / unique_filename
|
||||
# Define the path where the file will be saved
|
||||
file_path = os.path.join(settings.FILE_STORAGE_DIR, unique_filename)
|
||||
|
||||
# Check file size
|
||||
# First, we need to get the file size
|
||||
upload_file.file.seek(0, os.SEEK_END)
|
||||
file_size = upload_file.file.tell()
|
||||
upload_file.file.seek(0) # Reset file position
|
||||
# Check file size
|
||||
# First, we need to get the file size
|
||||
upload_file.file.seek(0, os.SEEK_END)
|
||||
file_size = upload_file.file.tell()
|
||||
upload_file.file.seek(0) # Reset file position
|
||||
|
||||
if file_size > settings.MAX_FILE_SIZE:
|
||||
if file_size > settings.MAX_FILE_SIZE:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE,
|
||||
detail=f"File size exceeds the maximum allowed size of {settings.MAX_FILE_SIZE} bytes",
|
||||
)
|
||||
|
||||
# Save the file
|
||||
with open(file_path, "wb") as buffer:
|
||||
shutil.copyfileobj(upload_file.file, buffer)
|
||||
|
||||
logger.info(f"File saved successfully at {file_path}")
|
||||
return unique_filename, file_path, file_size
|
||||
|
||||
except PermissionError as e:
|
||||
logger.error(f"Permission denied when saving file: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE,
|
||||
detail=f"File size exceeds the maximum allowed size of {settings.MAX_FILE_SIZE} bytes",
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Permission denied when saving file. The server may not have write access to the storage directory.",
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(f"Error saving file: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="An error occurred while saving the file",
|
||||
)
|
||||
|
||||
# Save the file
|
||||
with open(file_path, "wb") as buffer:
|
||||
shutil.copyfileobj(upload_file.file, buffer)
|
||||
|
||||
return unique_filename, str(file_path), file_size
|
||||
|
||||
@staticmethod
|
||||
def get_file_path(filename: str) -> Path:
|
||||
def get_file_path(filename: str) -> str:
|
||||
"""
|
||||
Get the full path for a file.
|
||||
|
||||
@ -59,9 +77,9 @@ class FileService:
|
||||
filename: The filename to retrieve
|
||||
|
||||
Returns:
|
||||
Path object pointing to the file
|
||||
String path to the file
|
||||
"""
|
||||
return settings.FILE_STORAGE_DIR / filename
|
||||
return os.path.join(settings.FILE_STORAGE_DIR, filename)
|
||||
|
||||
@staticmethod
|
||||
def file_exists(filename: str) -> bool:
|
||||
@ -75,7 +93,7 @@ class FileService:
|
||||
True if the file exists, False otherwise
|
||||
"""
|
||||
file_path = FileService.get_file_path(filename)
|
||||
return file_path.exists() and file_path.is_file()
|
||||
return os.path.exists(file_path) and os.path.isfile(file_path)
|
||||
|
||||
@staticmethod
|
||||
def delete_file(filename: str) -> bool:
|
||||
@ -88,11 +106,19 @@ class FileService:
|
||||
Returns:
|
||||
True if the file was deleted, False otherwise
|
||||
"""
|
||||
file_path = FileService.get_file_path(filename)
|
||||
if file_path.exists() and file_path.is_file():
|
||||
file_path.unlink()
|
||||
return True
|
||||
return False
|
||||
try:
|
||||
file_path = FileService.get_file_path(filename)
|
||||
if os.path.exists(file_path) and os.path.isfile(file_path):
|
||||
os.remove(file_path)
|
||||
logger.info(f"File deleted: {file_path}")
|
||||
return True
|
||||
return False
|
||||
except PermissionError as e:
|
||||
logger.error(f"Permission denied when deleting file: {e}")
|
||||
return False
|
||||
except Exception as e:
|
||||
logger.error(f"Error deleting file: {e}")
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def get_file_content(filename: str) -> BinaryIO:
|
||||
@ -108,11 +134,25 @@ class FileService:
|
||||
Raises:
|
||||
HTTPException: If the file does not exist
|
||||
"""
|
||||
file_path = FileService.get_file_path(filename)
|
||||
if not file_path.exists() or not file_path.is_file():
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="File not found",
|
||||
)
|
||||
try:
|
||||
file_path = FileService.get_file_path(filename)
|
||||
if not os.path.exists(file_path) or not os.path.isfile(file_path):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="File not found",
|
||||
)
|
||||
|
||||
return open(file_path, "rb")
|
||||
return open(file_path, "rb")
|
||||
|
||||
except PermissionError:
|
||||
logger.error(f"Permission denied when reading file: {filename}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Permission denied when reading file",
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(f"Error reading file {filename}: {e}")
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="An error occurred while reading the file",
|
||||
)
|
||||
|
Loading…
x
Reference in New Issue
Block a user