-
Notifications
You must be signed in to change notification settings - Fork 0
[Architecture Refactor][1] Base architecture implementation and user package refactor #46
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
base: refactor/architecture-base-branch
Are you sure you want to change the base?
Changes from all commits
8047fe6
22ded1e
64f070e
0ec008f
6244314
1f823a9
710f134
1e33fda
ff99d7d
64a7e7d
f3bd0a8
7db5a7c
24932e7
eece5a4
5a75255
7a1b6df
eee98e5
4a62b9d
d45f7cc
00451a1
78df8d0
4962d2a
35da134
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| from .session_dependency import * |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| from .entities import * | ||
| from .criteria import * | ||
| from .pagination_criteria import * | ||
| from .repository import * |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| __all__ = ("Criteria",) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make sense tu put all here if we only have 1 class to import?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's good python practice to have an Also, having an If a change is made in this file, the So, IMO, maintaining a simple |
||
|
|
||
|
|
||
| class Criteria[TEntity]: | ||
| pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| __all__ = ("Entity",) | ||
|
|
||
| from .entity import Entity |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| from __future__ import annotations | ||
|
|
||
| __all__ = ( | ||
| "EntityDefinitionError", | ||
| "MissingDtoError", | ||
| "InvalidDtoError", | ||
| ) | ||
|
|
||
| import typing as t | ||
|
|
||
| from app.common.exceptions import BaseApplicationError | ||
|
|
||
| from . import _utils | ||
|
|
||
| if t.TYPE_CHECKING: | ||
| from .entity import Entity | ||
| from .entity import _DtoConstraint | ||
|
|
||
|
|
||
| class EntityDefinitionError(BaseApplicationError): | ||
| pass | ||
|
|
||
|
|
||
| class MissingDtoError(EntityDefinitionError): | ||
| def __init__( | ||
| self, | ||
| entity_cls: type[Entity], | ||
| missing_dto_constraint: _DtoConstraint, | ||
| ) -> None: | ||
| dto_name, dto_cls = missing_dto_constraint | ||
| dto_bases = (dto_cls.__name__,) | ||
| model_name, model_bases = _utils.extract_class_info(entity_cls) | ||
|
|
||
| message_lines = ( | ||
| f"Missing {dto_name} definition from {model_name} declaration.", | ||
| "", | ||
| "Example:", | ||
| _utils.build_nested_class_definition_example( | ||
| outer_class_name=model_name, | ||
| outer_class_bases=model_bases, | ||
| inner_class_name=dto_name, | ||
| inner_class_bases=dto_bases, | ||
| inner_class_statements=( | ||
| "# fields required to perform operation via repository", | ||
| ), | ||
| ), | ||
| ) | ||
| message = "\n".join(message_lines) | ||
| super().__init__(message) | ||
|
|
||
|
|
||
| class InvalidDtoError(EntityDefinitionError): | ||
| def __init__( | ||
| self, | ||
| entity_cls: type[Entity], | ||
| invalid_dto_constraint: _DtoConstraint, | ||
| ) -> None: | ||
| dto_name, dto_cls = invalid_dto_constraint | ||
| dto_bases = (dto_cls.__name__,) | ||
| model_name, model_bases = _utils.extract_class_info(entity_cls) | ||
|
|
||
| message_lines = ( | ||
| f"Invalid {dto_name} schema definition, must be child of {dto_cls.__name__}", | ||
| "", | ||
| "Example:", | ||
| _utils.build_nested_class_definition_example( | ||
| outer_class_name=model_name, | ||
| outer_class_bases=model_bases, | ||
| inner_class_name=dto_name, | ||
| inner_class_bases=dto_bases, | ||
| inner_class_statements=( | ||
| "# fields required to perform operation via repository", | ||
| ), | ||
| ), | ||
| ) | ||
| message = "\n".join(message_lines) | ||
| super().__init__(message) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| __all__ = ( | ||
| "build_nested_class_definition_example", | ||
| "extract_class_info", | ||
| ) | ||
|
|
||
| import typing as t | ||
|
|
||
|
|
||
| def build_nested_class_definition_example( | ||
| *, | ||
| outer_class_name: str = "Outer", | ||
| outer_class_bases: t.Sequence[str] | None = None, | ||
| inner_class_name: str = "Inner", | ||
| inner_class_bases: t.Sequence[str] | None = None, | ||
| inner_class_statements: t.Sequence[str] = ("# ...",), | ||
| ) -> str: | ||
| output_lines = ( | ||
| _build_class_header(outer_class_name, outer_class_bases), | ||
| _indent(1, _build_class_header(inner_class_name, inner_class_bases)), | ||
| *_indent(2, inner_class_statements), | ||
| ) | ||
|
|
||
| return "\n".join(output_lines) | ||
|
|
||
|
|
||
| def _build_class_header(name: str, bases: t.Sequence[str] | None) -> str: | ||
| return f"class {name}{f'({", ".join(bases)})' if bases else ''}:" | ||
|
|
||
|
|
||
| @t.overload | ||
| def _indent(level: int, val: str) -> str: ... | ||
| @t.overload | ||
| def _indent(level: int, val: t.Sequence[str]) -> t.Sequence[str]: ... | ||
|
|
||
|
|
||
| def _indent(level: int, val: str | t.Sequence[str]) -> str | t.Sequence[str]: | ||
| space = " " * level | ||
|
|
||
| if isinstance(val, str): | ||
| return f"{space}{val}" | ||
|
|
||
| return tuple(f"{space}{line}" for line in val) | ||
|
|
||
|
|
||
| class _ClassInfo(t.NamedTuple): | ||
| class_name: str | ||
| class_bases: t.Sequence[str] | ||
|
|
||
|
|
||
| def extract_class_info(cls: type) -> _ClassInfo: | ||
| return _ClassInfo( | ||
| class_name=cls.__name__, | ||
| class_bases=tuple(base.__name__ for base in cls.__bases__), | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| __all__ = ("Entity",) | ||
|
|
||
| import typing as t | ||
|
|
||
| import pydantic | ||
|
|
||
| from . import _exc | ||
|
|
||
|
|
||
| class _DtoConstraint(t.NamedTuple): | ||
| name: str | ||
| parent_cls: type | ||
|
|
||
|
|
||
| type _DtoConstraints = t.Collection[_DtoConstraint] | ||
| type _DtoConstraintEvaluation = _exc.EntityDefinitionError | None | ||
| type _EntityDefinitionErrors = t.Sequence[_exc.EntityDefinitionError] | ||
|
|
||
|
|
||
| # TODO: schema registry and automatic model rebuilding (fix circular imports for good) | ||
| # TODO: sentinel for unloaded relationships (fix shape of data problem) | ||
| class Entity(pydantic.BaseModel): | ||
| """ | ||
| Base model of the application, representing an object in the db. | ||
|
|
||
| :var CreateDto: (required) Subschema for creating an object | ||
| :var UpdateDto: (required) Subschema for updating an object | ||
|
|
||
| ## Example: | ||
| ``` | ||
| class User(Entity): | ||
| id: UserId | ||
| name: str | ||
| last_name: str | ||
|
|
||
| class CreateDto(pydantic.BaseModel): | ||
| name: str | ||
| last_name: str | ||
|
|
||
| class UpdateDto(pydantic.BaseModel): | ||
| name: str | None = None | ||
| last_name: str | None = None | ||
| ``` | ||
| """ | ||
|
|
||
| model_config = pydantic.ConfigDict( | ||
| from_attributes=True, | ||
| frozen=True, | ||
| ) | ||
|
|
||
| CreateDto: t.ClassVar[type[pydantic.BaseModel]] | ||
| UpdateDto: t.ClassVar[type[pydantic.BaseModel]] | ||
|
Comment on lines
+51
to
+52
|
||
|
|
||
| @classmethod | ||
| def __pydantic_init_subclass__(cls, **kw: t.Any) -> None: | ||
| super().__pydantic_init_subclass__(**kw) | ||
| cls._validate_entity_definition() | ||
|
|
||
| __required_dtos__: t.Final[_DtoConstraints] = ( | ||
| _DtoConstraint("CreateDto", pydantic.BaseModel), | ||
| _DtoConstraint("UpdateDto", pydantic.BaseModel), | ||
| ) | ||
|
|
||
| @classmethod | ||
| def _validate_entity_definition(cls) -> None: | ||
| errors = cls._collect_entity_definition_errors() | ||
|
|
||
| if errors: | ||
| msg = f"Can't create {cls.__name__} entity class" | ||
| raise ExceptionGroup(msg, errors) | ||
|
|
||
| @classmethod | ||
| def _collect_entity_definition_errors(cls) -> _EntityDefinitionErrors: | ||
| errors = ( | ||
| cls._evaluate_dto_constraint(dto_constraint) | ||
| for dto_constraint in cls.__required_dtos__ | ||
| ) | ||
|
|
||
| return tuple(error for error in errors if error is not None) | ||
|
|
||
| @classmethod | ||
| def _evaluate_dto_constraint( | ||
| cls, | ||
| dto_constraint: _DtoConstraint, | ||
| ) -> _DtoConstraintEvaluation: | ||
| dto_name, dto_parent_cls = dto_constraint | ||
|
|
||
| if not hasattr(cls, dto_name): | ||
| return _exc.MissingDtoError(cls, dto_constraint) | ||
|
|
||
| dto = getattr(cls, dto_name) | ||
| is_type = isinstance(dto, type) | ||
| is_subclass = is_type and issubclass(dto, dto_parent_cls) | ||
|
|
||
| if not is_subclass: | ||
| return _exc.InvalidDtoError(cls, dto_constraint) | ||
|
|
||
| return None | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| __all__ = ("PaginationCriteria",) | ||
|
|
||
| import typing as t | ||
|
|
||
| import pydantic | ||
|
|
||
| from .criteria import Criteria | ||
|
|
||
| _PageField = t.Annotated[ | ||
| int, | ||
| pydantic.Field(ge=1), | ||
| ] | ||
| _PageSizeField = t.Annotated[ | ||
| int, | ||
| pydantic.Field(ge=1, le=100), | ||
| ] | ||
|
|
||
|
|
||
| class PaginationCriteria(pydantic.BaseModel, Criteria[t.Any]): | ||
| model_config = pydantic.ConfigDict(frozen=True) | ||
|
|
||
| page: _PageField = 1 | ||
| page_size: _PageSizeField = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any benefit on importing it like this? I know the import is clearer but I think the code is uglier 😄 . This is not a fix, just a preference.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, this is code style preference, yes, so it's very difficult (if not impossible) to have programmers agree on these kinds of discussions 😁.
Anyways, in short, for imports I follow the following pattern for (IMO) maximum readability:
__init__.py, similar to typescripts barrel files, that both exposes a clean interface, hides implementation details of the submodule that should not be imported outside of it, and allows other modules to have cleaner importsReasoning for these rules:
source -- symbolon every line and get a clear picture at a glance.Regarding the
import typing as t, I generally do it like that to have a short alias for a module that is (or should) be used a lot. It's the same reasoning why alembic imports sqlalchemy assa. It keeps the clarity of the above principles, helps with keeping line length short (again, important so the linter does not introduce weird artifacts that hurt readability), and makes typing the code easier. I generally always import it like that even if I require only one symbol from the typing module, in case the file grows later and more are needed, which creates better git diffs (as the imports are not shuffled around so much).Anyways, this is a summary of the benefits I considered when I came up with these. For me, in conjunction they make the code easier to work with. That said, If
import typing as tannoys you I can remove it.What do you think?