kenki0 / asyncapiservice Goto Github PK

1. С развитием проекта в тестах фикстур будет все больше и больше, и их нужно систематизировать. Можно вынести в отдельный файл (затем добавлять другие файлы, логически разделяя их) и тем самым облегчим conftest.py. Для этого в conftest.py есть pytest_plugins

2. В скриптах ожидания ES и Redis в тестах (tests/functional/utils/wait_for_es.py и tests/functional/utils/wait_for_redis.py) вместо вечного цикла, в который можно попасть при возникновении сложностей, лучше использовать backoff из прошлых спринтов. Он более функционален в сравнении с вечным циклом, теоретически он может сигнализировать команде поддержки после определенного количества попыток.

3. А вот эти файлы с какой целью у вас дублируются в тестах? tests/functional/es_mapping.py и tests/functional/index_fillings.py, их копии лежат в tests/functional/testdata

4. Соберите все фикстуры в одном месте, так будет легче их поддерживать в дальнейшем (например, тут и тут). Тут, как раз, пригодится п.1

5. Не используйте прямое указание кодов ответов сервера в тестах (200, 400 и т.д.), применяйте http.HTTPStatus. Этим можно немного повысить читаемость кода https://docs.python.org/3/library/http.html#http.HTTPStatus

6. В src/api/v1 много раз встречается вот такая конструкция в параметрах методов:

   page_num: int = Query(default=1, alias='page[number]', ge=1),
   page_size: int = Query(default=50, alias='page[size]', ge=1),

   Можно попробовать вынести эту пару в отдельный класс, какой-нибудь PaginatedParams, инжектить его в качестве зависимости в параметры методов и работать с ними как с объектом. Т.о. параметры пагинации будут в одном месте и при необходимости легко будет их изменить сразу и везде.

Проверить работу кеширования данных.

Оценка: 5

Ссылка на PR: pass

Покрыть функциональными тестами endpoint /search

Оценка: 8

Ссылка на PR: pass

Проверить класс по работе с хранилищем на SOLID. Изменить структуру программы, если это необходимо.

Оценка: 5

Ссылка на PR: pass

Добавить сервис, который обращается к Elasticsearch (используя драйвер) и возвращает информацию о фильмах – по одному или списком.

При запросе списком учитывать параметры фильтрации и сортировки.

Проверить класс по работе с бизнес-логикой на SOLID. Изменить структуру программы, если это необходимо.

Оценка: 8

Ссылка на PR: pass

Покрыть функциональными тестами endpoint /film

Оценка: 5

Ссылка на PR: pass

Проведите ревью задания своего коллеги по команде. Можно брать задания из спринта по ETL, и первого спринта из модуля "Сервис Async API".

Оценка: 2.5

Ссылка на PR: #43

Сделать ручку для инвалидации кеша. Логика работы следующая:

• ETL смотрит загрузились новые данные в ES или обновились
• Если да то он дергает нашу ручку, которая будет сбрасывать весь кеш

Так же реализовать авторизацию для ограничения доступа к этой ручке, самую простую через какой нибудь secret key который будет передаваться в headers запроса

Покрыть функциональными тестами endpoint /genre

Оценка: 5

Ссылка на PR: pass

Создать модели, прописав правильные типы данных для:

• фильмов,
• жанров,
• актеров, сценаристов, режиссеров.

Необходимо разработать инфраструктуру для запуска функциональных тестов. Тесты изолированно запускаются в docker-compose

Оценка: 13

Ссылка на PR: pass

Реализовать работу с внешними сервисами через exponential backoff

Оценка: 8

Ссылка на PR: pass

Собрать все фикстуры в одном месте. Реализовать pytest_plugins.

Оценка: 5

Ссылка на PR: pass

Исправление замечаний после ревъю

• А вот эти файлы с какой целью у вас дублируются в тестах?
• Не используйте прямое указание кодов ответов сервера в тестах (200, 400 и т.д.)
• В src/api/v1 много раз встречается вот такая конструкция в параметрах методов

Оценка: 1

Ссылка на PR: pass

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

При запросе списком учитывать параметры фильтрации и сортировки.

Оценка: 6.5

Хорошая работа 👍 Вы даже сделали часть следующего спринта.

1. В продакшене не рекомендуется запускать код через uvicorn напрямую, вот тут про это написано несколько слов https://www.uvicorn.org/deployment/#gunicorn.
2. В docker-compose.yaml не нужно пробрасывать порт Redis наружу. В части безопасности приложения это плохо.
3. Этот сервис выдачи контента, как и остальные сервисы кинотеатра, доступные пользователю, должен отдаваться через nginx.
4. А вы специально не наследовали класс Fillm, Genre и Person от DefaultModel? Orjson и все такое :)
5. В вашей версии python можно опциональные переменные и поля описывать немного проще,
   т.е. redis: Optional[Redis] можно представить как redis: Redis | None
6. Документация ваших сервисов выглядит максимально куцей, давайте добавим хотя бы description, подробнее тут: https://fastapi.tiangolo.com/tutorial/path-operation-configuration/?h=description#summary-and-description
7. В параметрах пагинации page_num: int = Query(default=1, alias='page[number]') и page_size: int = Query(default=50, alias='page[size]') рекомендую добавить валидацию, для исключения ввода отрицательных значений. Например: page_size: int = Query(default=50, alias='page[size]', ge=0) - это и в Swagger отразится.
8. Вот этот блок лишний.
9. Текстовые сообщения, отдаваемые пользователю, (film not found) у вас расположены в разных местах кода, если появится задача добавить информативности или изменить текст сообщения, или встанет вопрос про мультиязычность, то придется по всему проекту выискивать и менять. Будет проще справиться с этой задачей, если такие сообщения будут вынесены в отдельное место в виде констант (например).

Написать dockerfile для билда и написать docker compose, чтобы поднимать все сервисы, написанные ранее

Проведите ревью задания своего коллеги по команде. Можно брать задания из спринта по ETL, и первого спринта из модуля "Сервис Async API".

Оценка: 2.5

Ссылка на PR: #35

Проверить класс по работе с кешом на SOLID. Изменить структуру программы, если это необходимо.

Оценка: 5

Ссылка на PR: pass

Кешировать ответы ручек, запрошенных с одинаковыми параметрами, чтобы не ходить лишний раз в Elasticsearch.

Покрыть функциональными тестами endpoint /person

Оценка: 5

Ссылка на PR: pass

Добавить сервис, который обращается к Elasticsearch и возвращает информацию о персоналиях - по одному или списком.

При запросе списком учитывать параметры фильтрации и сортировки.

Оценка: 6.5

Подготовить документацию для клиентов сервиса, используя OpenAPI

Оценка: 5

Ссылка на PR: pass