Skip to content

Commit

Permalink
[DOP-14025] Remove pathlib.PurePosixPath from API schemas
Browse files Browse the repository at this point in the history
  • Loading branch information
dolfinus committed Apr 19, 2024
1 parent c4366f9 commit dce279f
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 36 deletions.
1 change: 1 addition & 0 deletions docs/changelog/next_release/41.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix 500 error while creating HDFS connection.
2 changes: 1 addition & 1 deletion syncmaster/backend/api/v1/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from syncmaster.backend.api.v1.connections import router as connection_router
from syncmaster.backend.api.v1.groups import router as group_router
from syncmaster.backend.api.v1.queue import router as queue_router
from syncmaster.backend.api.v1.transfers.router import router as transfer_router
from syncmaster.backend.api.v1.transfers import router as transfer_router
from syncmaster.backend.api.v1.users import router as user_router

router = APIRouter(prefix="/v1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
from kombu.exceptions import KombuError

from syncmaster.backend.api.deps import UnitOfWorkMarker
from syncmaster.backend.api.v1.transfers.utils import (
process_file_transfer_directory_path,
)
from syncmaster.backend.services import UnitOfWork, get_user
from syncmaster.db.models import Status, User
from syncmaster.db.utils import Permission
Expand Down Expand Up @@ -115,8 +112,6 @@ async def create_transfer(
if transfer_data.group_id != queue.group_id:
raise DifferentTransferAndQueueGroupError

transfer_data = process_file_transfer_directory_path(transfer_data) # type: ignore

async with unit_of_work:
transfer = await unit_of_work.transfer.create(
group_id=transfer_data.group_id,
Expand Down Expand Up @@ -316,8 +311,6 @@ async def update_transfer(
params_type=transfer_data.source_params.type,
)

transfer_data = process_file_transfer_directory_path(transfer_data) # type: ignore

async with unit_of_work:
transfer = await unit_of_work.transfer.update(
transfer=transfer,
Expand Down
2 changes: 0 additions & 2 deletions syncmaster/backend/api/v1/transfers/__init__.py

This file was deleted.

17 changes: 0 additions & 17 deletions syncmaster/backend/api/v1/transfers/utils.py

This file was deleted.

24 changes: 15 additions & 9 deletions syncmaster/schemas/v1/transfers/file/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@

from pathlib import PurePosixPath

from pydantic import BaseModel, Field, validator
from pydantic import BaseModel, Field, field_validator

from syncmaster.schemas.v1.transfers.file_format import CSV, JSON, JSONLine


def validate_directory_path(path: str) -> PurePosixPath:
return PurePosixPath(path)


# At the moment the ReadTransferSourceParams and ReadTransferTargetParams
# classes are identical but may change in the future
class ReadFileTransferSource(BaseModel):
Expand All @@ -28,20 +24,30 @@ class ReadFileTransferTarget(BaseModel):
# At the moment the CreateTransferSourceParams and CreateTransferTargetParams
# classes are identical but may change in the future
class CreateFileTransferSource(BaseModel):
directory_path: PurePosixPath
directory_path: str
file_format: CSV | JSONLine | JSON = Field(..., discriminator="type")

class Config:
arbitrary_types_allowed = True

_validate_dir_path = validator("directory_path", allow_reuse=True, pre=True)(validate_directory_path)
@field_validator("directory_path", mode="before")
@classmethod
def _directory_path_is_valid_path(cls, value):
if not PurePosixPath(value).is_absolute():
raise ValueError("Directory path must be absolute")
return value


class CreateFileTransferTarget(BaseModel):
directory_path: PurePosixPath
directory_path: str
file_format: CSV | JSONLine = Field(..., discriminator="type") # JSON FORMAT IS NOT SUPPORTED AS A TARGET !

class Config:
arbitrary_types_allowed = True

_validate_dir_path = validator("directory_path", allow_reuse=True, pre=True)(validate_directory_path)
@field_validator("directory_path", mode="before")
@classmethod
def _directory_path_is_valid_path(cls, value):
if not PurePosixPath(value).is_absolute():
raise ValueError("Directory path must be absolute")
return value
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,81 @@ async def test_developer_plus_can_create_hdfs_transfer(
"strategy_params": transfer.strategy_params,
"queue_id": transfer.queue_id,
}


@pytest.mark.parametrize(
"create_connection_data",
[
{
"type": "s3",
"host": "localhost",
"port": 443,
},
],
indirect=True,
)
@pytest.mark.parametrize(
"target_source_params",
[
{
"type": "s3",
"directory_path": "some/path",
"file_format": {
"type": "csv",
},
},
],
)
async def test_cannot_create_file_transfer_with_relative_path(
client: AsyncClient,
two_group_connections: tuple[MockConnection, MockConnection],
group_queue: Queue,
mock_group: MockGroup,
target_source_params: dict,
create_connection_data: dict,
):
# Arrange
first_connection, second_connection = two_group_connections
user = mock_group.get_member_of_role(UserTestRoles.Developer)

# Act
result = await client.post(
"v1/transfers",
headers={"Authorization": f"Bearer {user.token}"},
json={
"group_id": mock_group.group.id,
"name": "new test transfer",
"description": "",
"is_scheduled": False,
"schedule": "",
"source_connection_id": first_connection.id,
"target_connection_id": second_connection.id,
"source_params": target_source_params,
"target_params": target_source_params,
"strategy_params": {"type": "full"},
"queue_id": group_queue.id,
},
)

# Assert
assert result.status_code == 422
assert result.json() == {
"detail": [
{
"ctx": {"error": {}},
"input": "some/path",
"loc": ["body", "source_params", "s3", "directory_path"],
"msg": "Value error, Directory path must be absolute",
"type": "value_error",
"url": "https://errors.pydantic.dev/2.7/v/value_error",
},
{
"ctx": {"error": {}},
"input": "some/path",
"loc": ["body", "target_params", "s3", "directory_path"],
"msg": "Value error, Directory path must be absolute",
"type": "value_error",
"url": "https://errors.pydantic.dev/2.7/v/value_error",
},
],
}

0 comments on commit dce279f

Please sign in to comment.