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

Feature/db connect #179

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Feature/db connect #179

wants to merge 4 commits into from

Conversation

pullveryzator
Copy link
Collaborator

Написал crud для взаимодействия с базой. По сути сделал их общими для всех моделей. Любая критика и предложения приветствуются. Наверняка что-то упустил...

On branch feature/db_connect
Changes to be committed:
	new file:   src/app/core/__init__.py
	new file:   src/app/core/db.py
	new file:   src/app/crud/__init__.py
	new file:   src/app/crud/base.py
	new file:   src/app/crud/group.py
	new file:   src/app/crud/message.py
	new file:   src/app/crud/photo.py
	new file:   src/app/crud/user_tg.py
	new file:   src/app/models/__init__.py
	new file:   src/app/schemas/__init__.py
	modified:   src/bot/core/settings.py
On branch feature/db_connect
Changes to be committed:
	modified:   src/app/core/db.py
	modified:   src/app/crud/base.py
Changes to be committed:
	modified:   src/app/crud/base.py
Changes to be committed:
	modified:   src/app/core/db.py
	modified:   src/app/crud/group.py
	modified:   src/app/crud/message.py
	modified:   src/app/crud/photo.py
	modified:   src/app/crud/user_tg.py
Copy link
Contributor

@teamofroman teamofroman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Отлично.
Есть несколько моментов, которые надо поправить.

from app.models.models import UserTG


class CRUDUserTG(CRUDBase):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для пользователя надо переопределить метод create и update, т.к. в схеме будет поле password с паролем, а в БД мы храним его hash. Т.о. надо будет с словаре, который получится из схемы заменить password на hashed_password и вписать туда значение хешированного пароля.

"""Update object in database."""
obj_data = jsonable_encoder(db_obj)
update_data = pydantic_scheme_obj.dict(exclude_unset=True)
for field in obj_data:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай пойдем ноборот. Идем по update_data, проверяем, есть ли у db_obj аттрибут field (hasattr) и если есть, устанавливаем его значение.

session: AsyncSession) -> ModelType:
"""Update object in database."""
obj_data = jsonable_encoder(db_obj)
update_data = pydantic_scheme_obj.dict(exclude_unset=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай еще добавим исключение значений, которые установлены в None.

@@ -19,6 +19,13 @@ class Settings(BaseSettings):

email_curator: str = ''

postgres_user = ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай для единообразия кода добавим аннатации.
Значения по умолчанию убираем. Без этих данных мы не подключимся к БД.

postgres_server = ''
postgres_port = ''
database_url = ''

class Config: # noqa: D106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай добавим extra = 'ignore', что бы не валиться на тех настройках, которые есть в env-файле, но нет в классе.

"""Initilisation method for CRUDBase class."""
self.model = model

async def _get_by_attribute(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай сделаем так, что бы можно было делать фильтрацию по нескольким атрибутам. Все их соединяем по условию И

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants