Skip to content

Commit

Permalink
updating pydantic and bringing back missing tests (#402)
Browse files Browse the repository at this point in the history
* updating pydantic and bringing back missing tests

* removing tests for reconciliation_status fetching

the status field is deprecated and the tests are failing randomly
  • Loading branch information
defreng authored Sep 11, 2023
1 parent 60e39f5 commit e148bac
Show file tree
Hide file tree
Showing 20 changed files with 303 additions and 372 deletions.
199 changes: 156 additions & 43 deletions poetry.lock

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fastapi = "^0.103.1"
# Auxiliary
httpx = "^0.24.1"
tenacity = "^8.2.1"
pydantic = {version = "1.10.12", extras = ["dotenv"]}
pydantic = "^2.3.0"
structlog = "^23.1.0"
aiopath = "^0.6.10"

Expand All @@ -40,6 +40,7 @@ typer = "^0.9.0"
# Business
"ruamel.yaml" = "^0.17.20"
Jinja2 = "^3.0.3"
pydantic-settings = "^2.0.3"

[tool.poetry.dev-dependencies]
# Linting
Expand Down
26 changes: 11 additions & 15 deletions src/foxops/database/repositories/change.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from datetime import datetime, timezone
from typing import AsyncIterator, Self

from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict
from sqlalchemy import and_, delete, desc, insert, select, update
from sqlalchemy.exc import IntegrityError, NoResultFound
from sqlalchemy.ext.asyncio import AsyncEngine
Expand Down Expand Up @@ -56,13 +56,11 @@ class ChangeInDB(BaseModel):

merge_request_id: str | None
merge_request_branch_name: str | None

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)

@classmethod
def from_database_row(cls, obj) -> Self:
change_in_db = cls.from_orm(obj)
change_in_db = cls.model_validate(obj)
change_in_db.created_at = change_in_db.created_at.replace(tzinfo=timezone.utc)

return change_in_db
Expand All @@ -83,9 +81,7 @@ class IncarnationWithChangesSummary(BaseModel):
requested_version: str
merge_request_id: str | None
created_at: datetime

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)


class ChangeRepository:
Expand Down Expand Up @@ -141,7 +137,7 @@ async def create_change(
row = result.one()
await conn.commit()

return ChangeInDB.from_orm(row)
return ChangeInDB.model_validate(row)

async def create_incarnation_with_first_change(
self,
Expand Down Expand Up @@ -186,7 +182,7 @@ async def create_incarnation_with_first_change(
row = result.one()
await conn.commit()

return ChangeInDB.from_orm(row)
return ChangeInDB.model_validate(row)

async def delete_incarnation(self, id_: int) -> None:
async with self.engine.connect() as conn:
Expand Down Expand Up @@ -229,7 +225,7 @@ async def get_latest_change_for_incarnation(self, incarnation_id: int) -> Change
except NoResultFound:
raise IncarnationHasNoChangesError(incarnation_id)
else:
return ChangeInDB.from_orm(row)
return ChangeInDB.model_validate(row)

def _incarnations_with_changes_summary_query(self):
alias_change = change.alias("change")
Expand Down Expand Up @@ -274,7 +270,7 @@ async def list_incarnations_with_changes_summary(self) -> AsyncIterator[Incarnat

async with self.engine.connect() as conn:
for row in await conn.execute(query):
yield IncarnationWithChangesSummary.from_orm(row)
yield IncarnationWithChangesSummary.model_validate(row)

async def get_incarnation_by_repo_and_target_dir(
self, incarnation_repository: str, target_directory: str
Expand All @@ -291,7 +287,7 @@ async def get_incarnation_by_repo_and_target_dir(
except NoResultFound:
raise IncarnationNotFoundError(0)

return IncarnationWithChangesSummary.from_orm(row)
return IncarnationWithChangesSummary.model_validate(row)

async def list_changes(self, incarnation_id: int) -> list[ChangeInDB]:
query = select(change).where(change.c.incarnation_id == incarnation_id).order_by(desc(change.c.revision))
Expand Down Expand Up @@ -331,7 +327,7 @@ async def update_commit_sha(self, id_: int, commit_sha: str) -> ChangeInDB:
row = result.one()
await conn.commit()

return ChangeInDB.from_orm(row)
return ChangeInDB.model_validate(row)

async def update_commit_pushed(self, id_: int, commit_pushed: bool) -> ChangeInDB:
return await self._update_one(id_, commit_pushed=commit_pushed)
Expand All @@ -347,4 +343,4 @@ async def _update_one(self, id_: int, **kwargs) -> ChangeInDB:
row = result.one()
await conn.commit()

return ChangeInDB.from_orm(row)
return ChangeInDB.model_validate(row)
6 changes: 2 additions & 4 deletions src/foxops/database/repositories/incarnation/model.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict


class IncarnationInDB(BaseModel):
id: int
incarnation_repository: str
target_directory: str
template_repository: str

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)
6 changes: 3 additions & 3 deletions src/foxops/database/repositories/incarnation/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ async def create(
) from e

row = result.one()
return IncarnationInDB.from_orm(row)
return IncarnationInDB.model_validate(row)

async def list(self) -> AsyncIterator[IncarnationInDB]:
query = select(incarnations)

async with self.engine.begin() as conn:
for row in await conn.execute(query):
yield IncarnationInDB.from_orm(row)
yield IncarnationInDB.model_validate(row)

async def get_by_id(self, id_: int) -> IncarnationInDB:
query = select(incarnations).where(incarnations.c.id == id_)
Expand All @@ -61,7 +61,7 @@ async def get_by_id(self, id_: int) -> IncarnationInDB:
except NoResultFound:
raise IncarnationNotFoundError(f"could not find incarnation in DB with id: {id_}")
else:
return IncarnationInDB.from_orm(row)
return IncarnationInDB.model_validate(row)

async def delete_by_id(self, id_: int) -> None:
query = delete(incarnations).where(incarnations.c.id == id_)
Expand Down
17 changes: 7 additions & 10 deletions src/foxops/engine/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from pathlib import Path
from typing import Annotated, Mapping

from pydantic import BaseModel, Field
from pydantic import BaseModel, ConfigDict, Field
from ruamel.yaml import YAML

from foxops.logger import get_logger
Expand Down Expand Up @@ -70,12 +70,11 @@ class VariableDefinition(BaseModel):
Field(help="The default value for this variable (setting this makes the variable optional)"),
] = None

model_config = ConfigDict(frozen=True)

def is_required(self) -> bool:
return self.default is None

class Config:
allow_mutation = False


class TemplateRenderingConfig(BaseModel):
excluded_files: list[str] = Field(
Expand All @@ -84,8 +83,7 @@ class TemplateRenderingConfig(BaseModel):
"Glob patterns are supported, but must match the individual files. Use '**/*' to match everything recursively.",
)

class Config:
allow_mutation = False
model_config = ConfigDict(frozen=True)


class TemplateConfig(BaseModel):
Expand All @@ -112,6 +110,8 @@ class TemplateConfig(BaseModel):

variables: dict[str, VariableDefinition] = Field(default_factory=dict)

model_config = ConfigDict(frozen=True)

@property
def required_variables(self) -> dict[str, VariableDefinition]:
return {k: v for k, v in self.variables.items() if v.is_required()}
Expand All @@ -122,10 +122,7 @@ def optional_variables_defaults(self) -> TemplateData:

def to_yaml(self, target: Path) -> None:
with target.open("w") as f:
yaml.dump(self.dict(), f)

class Config:
allow_mutation = False
yaml.dump(self.model_dump(), f)


def load_template_config(template_config_path: Path) -> TemplateConfig:
Expand Down
8 changes: 4 additions & 4 deletions src/foxops/hosters/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def add(self, title: str, description: str, source_branch: str) -> int:
status=MergeRequestStatus.OPEN,
)

self._mr_file_path(mr.id).write_text(mr.json())
self._mr_file_path(mr.id).write_text(mr.model_dump_json())
self._next_id += 1

return mr.id
Expand All @@ -51,15 +51,15 @@ def delete(self, id_: int) -> None:
self._mr_file_path(id_).unlink()

def get(self, id_: int) -> MergeRequest:
return MergeRequest.parse_file(self._mr_file_path(id_))
return MergeRequest.model_validate_json(self._mr_file_path(id_).read_text())

def update_status(self, id_: int, status: MergeRequestStatus) -> None:
mr = self.get(id_)
mr.status = status
self._mr_file_path(mr.id).write_text(mr.json())
self._mr_file_path(mr.id).write_text(mr.model_dump_json())

def __iter__(self) -> Iterator[MergeRequest]:
yield from [MergeRequest.parse_file(p) for p in self.directory.glob("*.json")]
yield from [MergeRequest.model_validate_json(p.read_text()) for p in self.directory.glob("*.json")]

def _mr_file_path(self, id_: int) -> Path:
return self.directory / f"{id_}.json"
Expand Down
5 changes: 2 additions & 3 deletions src/foxops/models/desired_incarnation_state.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict

import foxops.engine as fengine

Expand All @@ -13,8 +13,7 @@ class DesiredIncarnationState(BaseModel):
template_data: fengine.TemplateData
automerge: bool = False

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)

def __eq__(self, other) -> bool:
if isinstance(other, fengine.IncarnationState):
Expand Down
11 changes: 4 additions & 7 deletions src/foxops/models/incarnation.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import Mapping

from pydantic import BaseModel, Field
from pydantic import BaseModel, ConfigDict, Field

from foxops.engine.models import TemplateDataValue
from foxops.hosters import ReconciliationStatus
Expand All @@ -17,8 +17,7 @@ class Incarnation(BaseModel):
commit_sha: str
merge_request_id: str | None

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)


class IncarnationBasic(BaseModel):
Expand All @@ -32,8 +31,7 @@ class IncarnationBasic(BaseModel):
merge_request_id: str | None
merge_request_url: str | None

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)


class IncarnationWithDetails(IncarnationBasic):
Expand All @@ -45,5 +43,4 @@ class IncarnationWithDetails(IncarnationBasic):
template_repository_version_hash: str | None
template_data: Mapping[str, TemplateDataValue] | None

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)
24 changes: 12 additions & 12 deletions src/foxops/routers/changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from typing import Annotated, Self

from fastapi import APIRouter, Depends, HTTPException, Path, status
from pydantic import BaseModel, root_validator
from pydantic import BaseModel, model_validator

from foxops.database.repositories.change import ChangeNotFoundError
from foxops.dependencies import get_change_service
Expand Down Expand Up @@ -34,16 +34,16 @@ class CreateChangeType(enum.Enum):


class CreateChangeRequest(BaseModel):
requested_version: str | None
requested_data: dict[str, str] | None
requested_version: str | None = None
requested_data: dict[str, str] | None = None
change_type: CreateChangeType = CreateChangeType.DIRECT

@root_validator
def check_either_version_or_data_change_requested(cls, values):
if values["requested_version"] is None and values["requested_data"] is None:
@model_validator(mode="after")
def check_either_version_or_data_change_requested(self) -> Self:
if self.requested_version is None and self.requested_data is None:
raise ValueError("Either requested_version or requested_data must be set")

return values
return self


class ChangeType(enum.Enum):
Expand All @@ -65,17 +65,17 @@ class ChangeDetails(BaseModel):
created_at: datetime
commit_sha: str

merge_request_id: str | None
merge_request_branch_name: str | None
merge_request_status: MergeRequestStatus | None
merge_request_id: str | None = None
merge_request_branch_name: str | None = None
merge_request_status: MergeRequestStatus | None = None

@classmethod
def from_service_object(cls, obj: Change | ChangeWithMergeRequest) -> Self:
match obj:
case ChangeWithMergeRequest():
return cls(type=ChangeType.MERGE_REQUEST, **obj.dict())
return cls(type=ChangeType.MERGE_REQUEST, **obj.model_dump())
case Change():
return cls(type=ChangeType.DIRECT, **obj.dict())
return cls(type=ChangeType.DIRECT, **obj.model_dump())
case _:
raise NotImplementedError(f"Unknown change type {type(obj)}")

Expand Down
2 changes: 1 addition & 1 deletion src/foxops/services/change.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ async def get_change_with_merge_request(self, change_id: int) -> ChangeWithMerge
)

return ChangeWithMergeRequest(
**change_basic.dict(),
**change_basic.model_dump(),
merge_request_id=change_in_db.merge_request_id,
merge_request_branch_name=change_in_db.merge_request_branch_name,
merge_request_status=status,
Expand Down
9 changes: 4 additions & 5 deletions src/foxops/services/incarnation.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict

from foxops.database.repositories.incarnation.repository import IncarnationRepository
from foxops.hosters import Hoster
Expand All @@ -10,8 +10,7 @@ class Incarnation(BaseModel):
target_directory: str
template_repository: str

class Config:
orm_mode = True
model_config = ConfigDict(from_attributes=True)


class IncarnationService:
Expand All @@ -36,11 +35,11 @@ async def create(
target_directory=target_directory,
template_repository=template_repository,
)
return Incarnation.from_orm(incarnation_in_db)
return Incarnation.model_validate(incarnation_in_db)

async def get_by_id(self, id_: int) -> Incarnation:
incarnation_in_db = await self.incarnation_repository.get_by_id(id_)
return Incarnation.from_orm(incarnation_in_db)
return Incarnation.model_validate(incarnation_in_db)

async def delete(self, incarnation: Incarnation) -> None:
await self.incarnation_repository.delete_by_id(incarnation.id)
Loading

0 comments on commit e148bac

Please sign in to comment.