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

Code review #29

Open
BigDeepBlue opened this issue Nov 9, 2022 · 3 comments
Open

Code review #29

BigDeepBlue opened this issue Nov 9, 2022 · 3 comments

Comments

@BigDeepBlue
Copy link
Collaborator

Молодцы, отличная работа 🔥 Все задачи реализованы, исследование проведено и оформлено на высоком уровне.

Рекомендации по ETL:

  1. Вот тут размер пачки по умолчанию лучше сделать побольше. И вот почему https://clickhouse.com/docs/ru/introduction/performance/#proizvoditelnost-pri-vstavke-dannykh
  2. А тут попробуйте добавить временную составляющую, на случай если у нас вдруг будет пауза при поступлении данных и батч не ждал бы полного наполнения.

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

    class MongoSettings(BaseSettings):
        host: str = ... # служебный символ, делающий поле обязательным
        port: int = ...
        class Config:
            env_prefix = 'mongo_'
    
  2. У вас в ugc_mongo_service/src/api/v1/movie.py кто угодно может проголосовать от имени кого угодно. Т.к. предполагается, что user_id будет в теле запроса. Это не совсем правильно. У нас есть сервис авторизации и нужно его использовать.

    • либо тут же в сервисе разбирайте JWT и извлекайте id - при таком подходе мы будем зависить от времени жизни токена, т.е. пользователь уже отключен, но до конца жизни токена будет пользоваться сервисом
    • либо сделайте middleware и переправляйте JWT в сервис авторизации и получайте назад все данные пользователя.
  3. И как всегда нужно добавить красок к Swagger документации сервиса. https://fastapi.tiangolo.com/tutorial/path-operation-configuration/?h=description#summary-and-description

  4. Вы на версии python 3.10. Начиная с версии python 3.9 для стандартных коллекций больше не нужен модуль typing: https://docs.python.org/3.9/whatsnew/3.9.html#type-hinting-generics-in-standard-collections. Поправьте во всем проекте.

  5. И немного про логирование. Вот интересная статься про lazy evaluation https://okomestudio.net/biboroku/2020/04/on-lazy-logging-evaluation/. Крайне рекомендую к применению

@alertdr
Copy link
Collaborator

alertdr commented Nov 9, 2022

Насчет
Вы на версии python 3.10. Начиная с версии python 3.9 для стандартных коллекций больше не нужен модуль typing: https://docs.python.org/3.9/whatsnew/3.9.html#type-hinting-generics-in-standard-collections. Поправьте во всем проекте.
К сожалению mypy не позволяет, тк в задании сказано, что проверка должна проходить для трех версий 3.7, 3.8, 3.9

@BigDeepBlue
Copy link
Collaborator Author

BigDeepBlue commented Nov 12, 2022

Суть задания про 3.7, 3.8, 3.9 - познакомиться с матрицей github Actions, там можно было поставить 3.10 и 3.11

@BigDeepBlue
Copy link
Collaborator Author

LGTM

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

No branches or pull requests

2 participants