Ревью в проекте
Closed this issue · 1 comments
aigt commented
Проведите ревью 2 заданий своего коллеги по команде. Можно брать задания из спринта по ETL, и первого спринта из модуля "Сервис Async API".
Оценка: 5
Ссылка на PR: pass
aigt commented
Ревью 2 заданий по ETL
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/etl.py
Информационный лог с уровнем warning
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/decorators/period.py
def wrapper(*args, **kwargs) -> Callable:
time.sleep(secs)
return func(*args, **kwargs)
- wrapper возвращает не Callable объект, а результат выполнения func(*args, **kwargs)
- Декоратор засыпает до выполнения функции, логичнее засыпать после выполнения
- wrapper лучше оборачивать в декоратор functools.wraps чтобы сохранять атрибуты оригинальной функции https://docs.python.org/3/library/functools.html#functools.wraps
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/decorators/es_reconnect.py
- Придирка: опечатка а -> в
"""Декоратор для поаторного соединения с ElasticSearch"""
^
- wrapper лучше оборачивать в декоратор functools.wraps чтобы сохранять атрибуты оригинальной функции https://docs.python.org/3/library/functools.html#functools.wraps
- Заявлено: Декоратор для поаторного соединения с ElasticSearch. По факту возвращает результат выполнения функции extractor, а если ES свалится, то выполнит только логирование и зарайзит ошибку - реконнекта выполнено не будет.
- Имя переменной extractor не соответствует тому как она используется в декораторе
- Информациооное сообщение логирования со статусом warning
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/decorators/pg_reconnect.py
Те же ошибки что и в es_reconnect.py
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/models/movie.py
- Придирка: библиотека pydantic подключена, датаклассы легко можнозаменить и использовать валидацию от пидантика:
from dataclasses import dataclass
Заменив на:
from pydantic.dataclasses import dataclass
- Более эффективное использование датаклассами памяти при включении флагов:
@dataclass(frozen=True, slots=True)
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/operations/extract.py
- connected возвращает bool или None и потом проверяется в if.
def connected(self) -> bool | None:
return self._connection and self._connection.closed == 0
Если self._connection
заменить на
self._connection is not None
Возвращаться будет только bool и проверка в if будет более правильной
- Название функции connect не соответствует заявленному функционалу: "Функция пересоздает соединение с БД"
- Придирка: проставлены не все аннотации типов
https://github.com/aigt/Async_API_sprint_2/blob/main/etl/src/states/state.py
Закомментирован неиспользуемый код