-
Notifications
You must be signed in to change notification settings - Fork 743
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
Potential Improvements #1
Comments
Hey, @ThirVondukr Thank you for your kind words, I truly appreciate them!
|
Storing db models in same place is just a personal preference, but I feel like it's very usable in small applications/microservices. You can use poetry in docker, either by exporting your dependencies as FROM python:3.10-slim as build
ENV POETRY_VERSION=1.1.14
RUN pip install poetry==$POETRY_VERSION
COPY ./pyproject.toml ./poetry.lock ./
RUN poetry export -f requirements.txt -o requirements.txt
FROM python:3.10-slim as final
WORKDIR /app
ENV PYTHONPATH=$PYTHONPATH:src
COPY --from=build requirements.txt .
RUN pip install --upgrade pip && pip install -r requirements.txt --no-cache-dir
COPY ./src ./src
COPY alembic.ini ./
... |
Yep, agree. We actually do store all DB models in one folder in a small service we have for transcoding. Well, your example with multi-stage looks pretty neat and clean! I didn't like the examples I have seen online, but will definitely try your example. |
I'd say there's also a "problem" with using sqlalchemy commits with fastapi. async def get_session() -> AsyncIterable[AsyncSession]:
async with async_sessionmaker.begin() as session:
yield session This could be avoided by either
I think that also could be covered in this repo |
I agree with the usage of poetry in combination with docker multi stage, this behavior is also suggested by official documentation. Another thing I usually do is to create a Then simply include it into the docker first stage:
|
I am using the method described here as a way to split secrets over different environments with pydantic settings: |
@b-bokma I'm currently using kubernetes and it's really easy to store your k8s secrets and configmaps in same format as your pydantic models: class DatabaseSettings(BaseSettings):
class Config:
env_prefix = "database_"
driver: str = "postgresql+asyncpg"
sync_driver = "postgresql+psycopg2"
name: str
username: str
password: str
host: str apiVersion: apps/v1
kind: Deployment
metadata:
...
spec:
...
template:
...
spec:
...
containers:
- name: fastapi
...
envFrom:
- secretRef:
name: app-database
prefix: database_ This also helps if you need to use same secret for different applications |
После того как была разработана архитектура эндпойнтов авторизации (#77), необходимо приступить к реализации этих эндпойнтов. Т.к. процессы аутентификации и авторизации требуют наличия зарегистрированного в системе Аккаунта, то в первую очередь нужно реализовать эндпойнт регистрации (`src/auth/routes.py :: create_account`). Планируемая архитектура бд: **Account** - id (primary key) - login (unique) - email (unique) - created_at - updated_at **PasswordHash** - account_id (foreign key, primary key) - value **Profile** - account_id (foreign key, primary key) - avatar (nullable, unique) - description (nullable) - name В рамках этой задачи необходимо написать реализацию для эндпойнта регистрации. --- В процессе реализации были приняты следующие решения: - **Использование foreign key в качестве primary key для таблиц, имеющих связь один к одному.** Например, Профиль пользователя всегда связан с одним и только одним Аккаунтом, таким образом в качестве primary key для таблицы Профиля будет использоваться foreign key на айди записи в таблице Аккаунта. Это позволит нам обеспечить соблюдение некоторых инвариантов доменной модели на уровне бд. - **Отказ от использования `default` и `onupdate` в колонках даты и времени.** Это решение было принято в качестве следствия из разработанного подхода работы с бд, а именно: "бд **должна** автоматизировать проверку как можно большего количества **ограничений**, но при этом бд **не должна** автоматизировать какое-либо **изменение** данных. Следование этому принципу позволит нам, во-первых, сохранить бизнес логику изменения данных на уровне кода приложения, т.е. сохранить её в явном виде, а не спрятать в бд в виде триггеров и процедур. А во-вторых, обеспечить поддержание целостности данных при попытке внесения изменений в обход кода нашего приложения (например, при проведении миграций). - **Расположение логики обработчиков ошибок в `src/app.py`.** Это решение является временным компромиссом и, возможно, будет изменено во время будущего рефакторинга. Мы приняли это решение в связи с тем, что fastapi позволяет привязывать обработчики ошибок только к экземпляру приложения, но не к отдельным роутам. - **Отказ от инкапсуляции логики создания зависимых моделей в методах основной модели.** Например, при создании Аккаунта всегда должен создаваться соответствующий аккаунту Профиль, но на данных момент эндпойнт создания Аккаунта не содержит в себе вызов эндпойнта создания Профиля. Это связано с тем, что после реализации MVP, мы планируем провести рефакторинг кода, связанного с внесением изменений в бд и созданием объектов модели, с помощью внедрения DDD репозиториев, фабрик и агрегатов. Наша "упрощенная" текущая реализация скорее всего позволит перейти на новый подход более безболезненно (по крайней мере сейчас так кажется). - **Сохранение таймзоны в бд и использование timezone-aware объектов даты и времени.** Это позволит нам всегда точно знать, что в нашей бд все записи даты и времени добавленные/измененные нашим приложением были сделаны с учетом таймзоны. - **Использование `.flush()` вместо `.commit()` в методах изменяющих данные в бд.** Все методы вносящие изменения в данные бд будут использовать `.flush()`, чтобы в нашем коде, в рамках сессии мы могли видеть данные в изменённом виде, но при этом `.commit()` будет вызываться только в тирдауне `get_db`, позволяя нам фиксировать изменения атомарно в рамках всего эндпойнта, а не в рамках вызовов отдельных методов. У это подхода тоже есть серьезный недостаток - в связи с особенностями работы `Dependency` в fastapi, применение изменений в бд будет происходить уже после того как ответ 200 будет отправлен (см [комментарий](zhanymkanov/fastapi-best-practices#1 (comment))). Но т.к. для решения этой проблемы необходимо проводить отдельный ресёрч, было принято решение исправить это в рамках [другой](#142) задачи.
После того как была разработана архитектура эндпойнтов авторизации (#77), необходимо приступить к реализации этих эндпойнтов. Т.к. процессы аутентификации и авторизации требуют наличия зарегистрированного в системе Аккаунта, то в первую очередь нужно реализовать эндпойнт регистрации (`src/auth/routes.py :: create_account`). Планируемая архитектура бд: **Account** - id (primary key) - login (unique) - email (unique) - created_at - updated_at **PasswordHash** - account_id (foreign key, primary key) - value **Profile** - account_id (foreign key, primary key) - avatar (nullable, unique) - description (nullable) - name В рамках этой задачи необходимо написать реализацию для эндпойнта регистрации. --- В процессе реализации были приняты следующие решения: - **Использование foreign key в качестве primary key для таблиц, имеющих связь один к одному.** Например, Профиль пользователя всегда связан с одним и только одним Аккаунтом, таким образом в качестве primary key для таблицы Профиля будет использоваться foreign key на айди записи в таблице Аккаунта. Это позволит нам обеспечить соблюдение некоторых инвариантов доменной модели на уровне бд. - **Отказ от использования `default` и `onupdate` в колонках даты и времени.** Это решение было принято в качестве следствия из разработанного подхода работы с бд, а именно: "бд **должна** автоматизировать проверку как можно большего количества **ограничений**, но при этом бд **не должна** автоматизировать какое-либо **изменение** данных. Следование этому принципу позволит нам, во-первых, сохранить бизнес логику изменения данных на уровне кода приложения, т.е. сохранить её в явном виде, а не спрятать в бд в виде триггеров и процедур. А во-вторых, обеспечить поддержание целостности данных при попытке внесения изменений в обход кода нашего приложения (например, при проведении миграций). - **Расположение логики обработчиков ошибок в `src/app.py`.** Это решение является временным компромиссом и, возможно, будет изменено во время будущего рефакторинга. Мы приняли это решение в связи с тем, что fastapi позволяет привязывать обработчики ошибок только к экземпляру приложения, но не к отдельным роутам. - **Отказ от инкапсуляции логики создания зависимых моделей в методах основной модели.** Например, при создании Аккаунта всегда должен создаваться соответствующий аккаунту Профиль, но на данных момент эндпойнт создания Аккаунта не содержит в себе вызов эндпойнта создания Профиля. Это связано с тем, что после реализации MVP, мы планируем провести рефакторинг кода, связанного с внесением изменений в бд и созданием объектов модели, с помощью внедрения DDD репозиториев, фабрик и агрегатов. Наша "упрощенная" текущая реализация скорее всего позволит перейти на новый подход более безболезненно (по крайней мере сейчас так кажется). - **Сохранение таймзоны в бд и использование timezone-aware объектов даты и времени.** Это позволит нам всегда точно знать, что в нашей бд все записи даты и времени добавленные/измененные нашим приложением были сделаны с учетом таймзоны. - **Использование `.flush()` вместо `.commit()` в методах изменяющих данные в бд.** Все методы вносящие изменения в данные бд будут использовать `.flush()`, чтобы в нашем коде, в рамках сессии мы могли видеть данные в изменённом виде, но при этом `.commit()` будет вызываться только в тирдауне `get_db`, позволяя нам фиксировать изменения атомарно в рамках всего эндпойнта, а не в рамках вызовов отдельных методов. У это подхода тоже есть серьезный недостаток - в связи с особенностями работы `Dependency` в fastapi, применение изменений в бд будет происходить уже после того как ответ 200 будет отправлен (см [комментарий](zhanymkanov/fastapi-best-practices#1 (comment))). Но т.к. для решения этой проблемы необходимо проводить отдельный ресёрч, было принято решение исправить это в рамках [другой](#142) задачи.
I don't understand why not to use poetry right in docker? The workflow is the following:
|
@PashaDem Because I personally don't think you need an extra dependency/tool inside of your docker container, if you need it for something - add it to your docker image. |
First of all - it's an amazing set of best practices, I also want to share some things that I use:
Project Structure
src
could be added toPYTHONPATH
to avoidprefixing every app import with
src
, IDEs like PyCharm also support that.Models also could be stored in same package, it's easier to import your models and make sure all modules were executed when you generate your migrations:
Continuous Integration
Absolutely use CI in gitlab/github to automate your tests and linters!
Dependency Management
Use poetry instead of requirements.txt, it's awesome!
Custom base model from day 0
This could also be used to enable
orm_mode
and set up custom
alias_generator
if client (for example a JS app) requires it.Use
Starlette's Config objectPydantic BaseSettings!Pydantic has its own class to manage environment variables:
The text was updated successfully, but these errors were encountered: