dondublon / graduate_work Goto Github PK

Использовать bookmarks из UGC.
Выдача как в спринте 4, с названием, актёрами и прочими работавшими над фильмом.
Переименовать в Favorites.

Тут действительно не стоит писать чувствительную информацию в логи.

Здесь не понял зачем строка с фастапи, ведь в сервисе оно никак не используется.

Сейчас он принимает только login+password, надо добавить auth token.

Тут почему бы не использовать ResponseModel-и вспомните спринты по фастапи вначале там были такие штуки

import orjson
from pydantic import BaseModel

def orjson_dumps(v, *, default):
"""Fast json dump implementation."""

return orjson.dumps(v, default=default).decode()

class Base(BaseModel):
"""Base model for representations of Elasticsearch data."""

uuid: str

class Config:
    json_loads = orjson.loads
    json_dumps = orjson_dumps

class FilmInfoResponse(Base):
"""Full film info response."""

title: str
description: str
imdb_rating: float

...

https://fastapi.tiangolo.com/tutorial/response-model/

gRPC. То есть ручку переделать.
переработать существующую регистрацию (в сервисе аутентифиации), чтобы писалось в профили, что надо.

Тут также не нашел код который закрывает соединение, думаю есть смысл сделать какой то get_connection() который вернет соединение а вы в коде будете использовать контекстный менеджер - https://www.psycopg.org/psycopg3/docs/basic/usage.html третья версия умеет закрывать соединение по выходе. Также думаю есть смысл использовать контекстный менеджер при работе с курсором (посмотрите вот пример есть сразу в доке).

S3 compatible, вроде.

Блокирует #5.

Тут думаю можно убрать if

Базовый вариант - локально

Схема С4 уровень 3.

Например, если не нашли нужного юзера на get-profile.
Хорошо бы получить контекст, там есть статус, но непонятно, как.

с обычной аутентификацией
Видимо, через обычный бекенд.
gRPC

19. Тут используйте какой нибудь асинхронный клиент вроде https://docs.aiohttp.org/en/stable/client.html или https://www.python-httpx.org/ иначе сейчас вызовы блокирующие основной цикл событий.

Использовать лайки.

Здесь не нашел где вызывается эта функция, определение есть, а вызова нет.

Тут значение success ведь не меняется, может так код будет нагляднее (и строк меньше) -

user_uuid = (await check_auth(request, authorize)).user_uuid
try:
    result = await Bookmarks.add(user_uuid, bookmark)
    logger.info("Successfully added %s, user=%s, %s=%s", COLLECTION_NAME, user_uuid, COLLECTION_NAME, bookmark)
    return orjson.dumps({"success": True, "inserted_id": str(result.inserted_id)})
except Exception as e:
    logger.error(
        "Error adding %s, user=%s, %s=%s, error=%s", COLLECTION_NAME, user_uuid, COLLECTION_NAME, bookmark, e
    )
    raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail=str(e))

Тут я вижу попытку запускать с помощью asyncio, но есть момент - тогда нужно весь

graduate_work/profiles/src/profiles_server.py

Line 18 in 6dac4d4
class Profiles(profiles_pb2_grpc.ProfilesServicer):
разработать с учетом того что он будет использовать асинхронную модель. Фактически использовать асинхронную модель работы с БД, а для этого sqlalchemy должна использоваться соотв способом - https://docs.sqlalchemy.org/en/20/orm/extensions/asyncio.html. А в варианте том как сейчас - получается вы запускаете с помощью asyncio синхронный код (тут обычный синхронный). Тут сами посмотрите примите решение оставить просто синхронный вариант или переписать с учетом исп с asyncio.

17. Тут есть шанс, что если код выше свалится - соединение не закроется. Используйте try-except-finally и закрывайте в finally.

Использовать reviews.

Тут лучше убрать отладочный принт, и я бы посоветовал принять меры по обеспечению отказоустойчивости, например не допускать падения сервиса при возникновении исключений, например когда кратковременно отвалился реббит, а вы туда пишете.

18. Тут немножечко не понял почему paginate ? Ожидал тут увидеть нечто задействованное в пагинации )

Генерировать реалистчные номера - видимо, с помощью библиотеки phonenumber. C её помощью sqlalchemy проверяет номер на валидность, сейчас случайный (из теста) не проходит.

Тут немного непонятно зачем класс.

Тут и тут похоже на дублирование

Нужно в т. ч. и для интеграционных тестов.

Хранить ФИО, email и номер телефона.

20. Тут и много где по коду употребляются одинарные и двойные кавычки - обычно договариваются об одном стиле. Я бы советовал прикрутить вот такую штуку - https://black.readthedocs.io/en/stable/ и прогнать файлики проекта. (а если прикрутить ее например в pre-commit - легко соблюдать единый стиль всей команды)

16. Тут вы использовали синхронную версию, посмотрите на https://aio-pika.readthedocs.io/en/latest/index.html иначе сейчас вызовы к реббиту у вас блокирующие.

Тут возможно стоит подумать про закрытие соединения по окончанию работы.

21. Тут проверьте, что пагинация действительно работает на этом результате. Я почему спросил - вот документация https://uriyyo-fastapi-pagination.netlify.app/tutorials/page-number-pagination/ и в примере последовательность это список моделей users: List[UserOut] и они же указаны в Page[UserOut], а у вас get_profiles по аннотации вернет непонятно пока что ), но возможно словари и лучше проверьте что это работает. На самом деле не очень эффективная штука так как насколько я понял оно отдаст страницу из всей последовательности, а всю последовательность ей необходимо держать в памяти.

Please, be careful when you work with databases, because default paginate will require to load all data in memory.

Здесь в админке можно добавить переводы + добавить в сборку компиляцию переводов - compilemessages

Здесь наверное случайно осталась строчка с инициализацией фастапи, не думаю что он нужен тут.

Сейчас это осложняется тем, что генерируемые файла импортируют прямо из корня. Посмотреть, как сделано в примере у Романа.

Здравствуйте ! Масштабная работа ! Есть небольшие предложения:

1. Здесь наверное случайно осталась строчка с инициализацией фастапи, не думаю что он нужен тут.
2. Здесь не нашел где вызывается эта функция, определение есть, а вызова нет.
3. Тут действительно не стоит писать чувствительную информацию в логи.
4. Здесь не понял зачем строка с фастапи, ведь в сервисе оно никак не используется.
5. Тут я вижу попытку запускать с помощью asyncio, но есть момент - тогда нужно весь

   graduate_work/profiles/src/profiles_server.py

   Line 18 in 6dac4d4

   class Profiles(profiles_pb2_grpc.ProfilesServicer):

   разработать с учетом того что он будет использовать асинхронную модель. Фактически использовать асинхронную модель работы с БД, а для этого sqlalchemy должна использоваться соотв способом - https://docs.sqlalchemy.org/en/20/orm/extensions/asyncio.html. А в варианте том как сейчас - получается вы запускаете с помощью asyncio синхронный код (тут обычный синхронный). Тут сами посмотрите примите решение оставить просто синхронный вариант или переписать с учетом исп с asyncio.
6. Тут можно использовать sessionmaker - https://docs.sqlalchemy.org/en/20/orm/session_basics.html#basics-of-using-a-session и далее в коде пользовали бы просто with Session() as session:...
7. Здесь в админке можно добавить переводы + добавить в сборку компиляцию переводов - compilemessages
8. Тут лучше убрать отладочный принт, и я бы посоветовал принять меры по обеспечению отказоустойчивости, например не допускать падения сервиса при возникновении исключений, например когда кратковременно отвалился реббит, а вы туда пишете.
9. Тут думаю можно убрать if
10. Тут и тут похоже на дублирование
11. Тут возможно стоит подумать про закрытие соединения по окончанию работы.
12. Тут также не нашел код который закрывает соединение, думаю есть смысл сделать какой то get_connection() который вернет соединение а вы в коде будете использовать контекстный менеджер - https://www.psycopg.org/psycopg3/docs/basic/usage.html третья версия умеет закрывать соединение по выходе. Также думаю есть смысл использовать контекстный менеджер при работе с курсором (посмотрите вот пример есть сразу в доке).
13. Тут немного непонятно зачем класс.
14. Тут значение success ведь не меняется, может так код будет нагляднее (и строк меньше) -

    user_uuid = (await check_auth(request, authorize)).user_uuid
    try:
        result = await Bookmarks.add(user_uuid, bookmark)
        logger.info("Successfully added %s, user=%s, %s=%s", COLLECTION_NAME, user_uuid, COLLECTION_NAME, bookmark)
        return orjson.dumps({"success": True, "inserted_id": str(result.inserted_id)})
    except Exception as e:
        logger.error(
            "Error adding %s, user=%s, %s=%s, error=%s", COLLECTION_NAME, user_uuid, COLLECTION_NAME, bookmark, e
        )
        raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail=str(e))

15. Тут почему бы не использовать ResponseModel-и вспомните спринты по фастапи вначале там были такие штуки

import orjson
from pydantic import BaseModel

def orjson_dumps(v, *, default):
    """Fast json dump implementation."""

    return orjson.dumps(v, default=default).decode()

class Base(BaseModel):
    """Base model for representations of Elasticsearch data."""

    uuid: str

    class Config:
        json_loads = orjson.loads
        json_dumps = orjson_dumps

class FilmInfoResponse(Base):
    """Full film info response."""

    title: str
    description: str
    imdb_rating: float
  ...

https://fastapi.tiangolo.com/tutorial/response-model/
16. Тут вы использовали синхронную версию, посмотрите на https://aio-pika.readthedocs.io/en/latest/index.html иначе сейчас вызовы к реббиту у вас блокирующие.
17. Тут есть шанс, что если код выше свалится - соединение не закроется. Используйте try-except-finally и закрывайте в finally.
18. Тут немножечко не понял почему paginate ? Ожидал тут увидеть нечто задействованное в пагинации )
19. Тут используйте какой нибудь асинхронный клиент вроде https://docs.aiohttp.org/en/stable/client.html или https://www.python-httpx.org/ иначе сейчас вызовы блокирующие основной цикл событий.
20. Тут и много где по коду употребляются одинарные и двойные кавычки - обычно договариваются об одном стиле. Я бы советовал прикрутить вот такую штуку - https://black.readthedocs.io/en/stable/ и прогнать файлики проекта. (а если прикрутить ее например в pre-commit - легко соблюдать единый стиль всей команды)
21. Тут проверьте, что пагинация действительно работает на этом результате. Я почему спросил - вот документация https://uriyyo-fastapi-pagination.netlify.app/tutorials/page-number-pagination/ и в примере последовательность это список моделей users: List[UserOut] и они же указаны в Page[UserOut], а у вас get_profiles по аннотации вернет непонятно пока что ), но возможно словари и лучше проверьте что это работает. На самом деле не очень эффективная штука так как насколько я понял оно отдаст страницу из всей последовательности, а всю последовательность ей необходимо держать в памяти.

Please, be careful when you work with databases, because default paginate will require to load all data in memory.

и "подружить" его бекенд с тем, что есть.

Тут можно использовать sessionmaker - https://docs.sqlalchemy.org/en/20/orm/session_basics.html#basics-of-using-a-session и далее в коде пользовали бы просто with Session() as session:...