Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added upload file funcitonality #22

Merged
merged 9 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
833 changes: 815 additions & 18 deletions poetry.lock

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ python-multipart = "^0.0.6"
boto3 = "^1.33.12"
alembic = "^1.12.0"
async-lru = "^2.0.4"
fsspec = "^2024.2.0"
s3fs = "^2024.2.0"

[tool.poetry.group.dev.dependencies]
pytest = "^7.4.3"
Expand All @@ -28,6 +30,7 @@ pytest-mock = "^3.12.0"
pytest-env = "^1.1.3"
pytest-alembic = "^0.10.7"
pytest-asyncio = "^0.23.2"
moto = "^4.2.13"
aiosqlite = "^0.19.0"
httpx = "^0.26.0"

Expand Down Expand Up @@ -68,6 +71,8 @@ env = [
"TOKEN_URL=http://localhost",
"CERTS_URL=http://localhost",
"AUTH_CLIENT=",
"UPLOAD_FS_PROTOCOL=file",
"UPLOAD_FS_ROOT=../upload",
]
testpaths = ["tests"]

Expand Down
17 changes: 16 additions & 1 deletion src/.env.local
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,19 @@ DB_NAME=filing
DB_USER=filing_user
DB_PWD=filing_user
DB_HOST=localhost:5432
DB_SCHEMA=filing
DB_SCHEMA=filing
KC_URL=http://localhost:8880
KC_REALM=regtech
KC_ADMIN_CLIENT_ID=admin-cli
KC_ADMIN_CLIENT_SECRET=local_test_only
KC_REALM_URL=${KC_URL}/realms/${KC_REALM}
KC_REALM_ADMIN_URL=${KC_URL}/admin/realms/${KC_REALM}
AUTH_URL=${KC_REALM_URL}/protocol/openid-connect/auth
AUTH_CLIENT=regtech-client
TOKEN_URL=${KC_REALM_URL}/protocol/openid-connect/token
CERTS_URL=${KC_REALM_URL}/protocol/openid-connect/certs
JWT_OPTS_VERIFY_AT_HASH="false"
JWT_OPTS_VERIFY_AUD="false"
JWT_OPTS_VERIFY_ISS="false"
UPLOAD_FS_PROTOCOL=file
UPLOAD_FS_ROOT=../upload
14 changes: 13 additions & 1 deletion src/.env.template
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,16 @@ DB_USER=
DB_PWD=
DB_HOST=
DB_SCHEMA=
# DB_SCHEME= can be used to override postgresql+asyncpg if needed
# DB_SCHEME= can be used to override postgresql+asyncpg if needed
KC_URL=
KC_REALM=
KC_ADMIN_CLIENT_ID=
KC_ADMIN_CLIENT_SECRET=
KC_REALM_URL=${KC_URL}/realms/${KC_REALM}
KC_REALM_ADMIN_URL=${KC_URL}/admin/realms/${KC_REALM}
AUTH_URL=${KC_REALM_URL}/protocol/openid-connect/auth
AUTH_CLIENT=
TOKEN_URL=${KC_REALM_URL}/protocol/openid-connect/token
CERTS_URL=${KC_REALM_URL}/protocol/openid-connect/certs
UPLOAD_FS_PROTOCOL=file
UPLOAD_FS_ROOT=../upload
18 changes: 18 additions & 0 deletions src/config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from enum import StrEnum
import os
from urllib import parse
from typing import Any
Expand All @@ -13,6 +14,11 @@
env_files_to_load.append(".env.local")


class FsProtocol(StrEnum):
FILE = "file"
S3 = "s3"


class Settings(BaseSettings):
db_schema: str = "public"
db_name: str
Expand All @@ -21,6 +27,18 @@ class Settings(BaseSettings):
db_host: str
db_scheme: str = "postgresql+asyncpg"
conn: PostgresDsn | None = None
"""
upload_fs_protocol: to be used with fsspec, and s3fs
`file` is for local file system
`s3` is for AWS S3
"""
upload_fs_protocol: FsProtocol = FsProtocol.FILE
"""
upload_fs_root: root of the upload folder in file system
with `file` protocol, this can be any directory you specific (e.g. `../upload`)
if using `s3` for the protocol, this should be the bucket name (e.g. `my-s3-bucket`)
"""
upload_fs_root: str

def __init__(self, **data):
super().__init__(**data)
Expand Down
2 changes: 1 addition & 1 deletion src/routers/filing.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ async def upload_file(
request: Request, lei: str, submission_id: str, file: UploadFile, background_tasks: BackgroundTasks
):
content = await file.read()
await submission_processor.upload_to_storage(lei, submission_id, content)
await submission_processor.upload_to_storage(lei, submission_id, content, file.filename.split(".")[-1])
background_tasks.add_task(submission_processor.validate_submission, lei, submission_id, content)


Expand Down
21 changes: 18 additions & 3 deletions src/services/submission_processor.py
lchen-2101 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
async def upload_to_storage(lei: str, submission_id: str, content: bytes):
# implement uploading process here
pass
from http import HTTPStatus
from fastapi import HTTPException
import logging
from fsspec import AbstractFileSystem, filesystem
from config import settings

log = logging.getLogger(__name__)


async def upload_to_storage(lei: str, submission_id: str, content: bytes, extension: str = "csv"):
try:
fs: AbstractFileSystem = filesystem(settings.upload_fs_protocol.value)
fs.mkdirs(f"{settings.upload_fs_root}/{lei}", exist_ok=True)
with fs.open(f"{settings.upload_fs_root}/{lei}/{submission_id}.{extension}", "wb") as f:
f.write(content)
except Exception as e:
log.error("Failed to upload file", e, exc_info=True, stack_info=True)
raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to upload file")

Copy link
Contributor

Choose a reason for hiding this comment

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

If the SubmissionDAO is getting created prior to this possible exception, what would happen to that object? Should we remove it from the database, or set a state on it? Originally the SUBMISSION_UPLOADED was going to be set after saving to S3, but since the object will be created before it now, would it just stay in that state and from that we can infer something bad happened (because it didn't move to VALIDATION_IN_PROGRESS)? This is for #52 really, but wanted to get thoughts so that story is implemented correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we'll be setting a state on it; so it's an additional one of upload failed, which I don't think we have yet.


async def validate_submission(lei: str, submission_id: str, content: bytes):
Expand Down
37 changes: 37 additions & 0 deletions tests/services/test_submission_processor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
from fastapi import HTTPException
import pytest
from unittest.mock import Mock, ANY
from pytest_mock import MockerFixture
from services.submission_processor import upload_to_storage


@pytest.fixture
def mock_fs(mocker: MockerFixture) -> Mock:
fs_mock_patch = mocker.patch("services.submission_processor.AbstractFileSystem")
return fs_mock_patch.return_value


@pytest.fixture
def mock_fs_func(mocker: MockerFixture, mock_fs: Mock) -> Mock:
fs_func_mock = mocker.patch("services.submission_processor.filesystem")
fs_func_mock.return_value = mock_fs
return fs_func_mock


async def test_upload(mocker: MockerFixture, mock_fs_func: Mock, mock_fs: Mock):
with mocker.mock_open(mock_fs.open):
await upload_to_storage("test", "test", b"test content")
mock_fs_func.assert_called()
mock_fs.mkdirs.assert_called()
mock_fs.open.assert_called_with(ANY, "wb")
file_handle = mock_fs.open()
file_handle.write.assert_called_with(b"test content")


async def test_upload_failure(mocker: MockerFixture, mock_fs_func: Mock, mock_fs: Mock):
log_mock = mocker.patch("services.submission_processor.log")
mock_fs.mkdirs.side_effect = IOError("test")
with pytest.raises(Exception) as e:
await upload_to_storage("test", "test", b"test content")
log_mock.error.assert_called_with("Failed to upload file", ANY, exc_info=True, stack_info=True)
assert isinstance(e.value, HTTPException)
3 changes: 0 additions & 3 deletions tests/test_test.py

This file was deleted.

Loading