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

[DOP-14025] Remove pathlib.PurePosixPath from API schemas #41

Merged
merged 1 commit into from
Apr 22, 2024
Merged
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
[DOP-14025] Remove pathlib.PurePosixPath from API schemas
dolfinus committed Apr 19, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 64e22906927e90aaf8c742ab53361e705be4ecc0
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
@@ -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")
Original file line number Diff line number Diff line change
@@ -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
@@ -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,
@@ -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,
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
@@ -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):
@@ -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
4 changes: 2 additions & 2 deletions tests/test_unit/test_transfers/test_create_transfer.py
Original file line number Diff line number Diff line change
@@ -521,7 +521,7 @@ async def test_developer_plus_cannot_create_transfer_with_other_group_queue(
}


async def test_developer_plus_can_not_create_transfer_with_target_s3_json(
async def test_developer_plus_can_not_create_transfer_with_target_format_json(
client: AsyncClient,
two_group_connections: tuple[MockConnection, MockConnection],
session: AsyncSession,
@@ -548,7 +548,7 @@ async def test_developer_plus_can_not_create_transfer_with_target_s3_json(
"source_params": {"type": "postgres", "table_name": "source_table"},
"target_params": {
"type": "s3",
"directory_path": "some/dir",
"directory_path": "/some/dir",
"df_schema": {},
"options": {},
"file_format": {
Original file line number Diff line number Diff line change
@@ -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",
},
],
}