Code review (sprint 7)

[BigDeepBlue]

Здравствуйте, отлично поработали ! Есть несколько рекомендаций:

1. Здесь у вас присутствуют миграции, но сами миграции пустые, не делают ничего. Неплохо было бы создать initial миграцию, которая создает все необходимые таблицы в БД (можно также посмотреть на Flask-Migrate сам его использую, достаточно удобно и легко прикручиваются миграции).
2. Здесь можно вынести значения в файл настроек, чтоб можно было управлять не поправляя код, если понадобится.
3. Тут неиспользуемый импорт, тут не хватает пустой строки, в конце файла много пустых строк. Могу посоветовать установить утилитку black - прогнать все файлы проекта, очень помогает избегать рутины с соблюдением pep8. Потому что много где вижу подобные отступления от pep8.
4. Здесь аннотация возвращаемого методом значения не соответствует действительности. Вы возвращаете tuple во всех случаях, т.е. верно tuple[Response, HTTPStatus], присмотритесь так же к аннотациям остальных методов, возможно там также нужны правки. Советую Вам попробуйте установите mypy https://mypy-lang.org - проведите проверку и сразу увидите где что получилось упустили или указали неверно. Очень помогает эта штука.
5. Здесь хочется заметить такую вещь, Вы действительно неограничены в решении как будут выглядеть пути ваших API endpoints, например Вы решили сделать единой точкой CRUD операции и далее в обработчике уже в зависимости от типа запроса выполняете логику. То как вы сделали не ошибка, но на практике может привести к тому что RoleServ.role_crud() внутри разрастется до больших размеров, и его будет трудно поддерживать.
6. Здесь и здесь по сути Вы дублируете в пути глагол типа операции, ведь в типе запроса уже заложен смысл какого то действия (создание, удаление). Думаю лаконичнее и достаточный для понимания клиентом вариант был бы например просто ..route('/user/roles', method=['DELETE'] или .route('/user/roles', method=['GET']) чтоб получить список ролей пользователя (то как Вы сделали не ошибка, этот пункт скорее для понимания что не стоит дублировать даже вот в таких местах).
7. Здесь также несоотвествие типа str , а метод ожидает dict.
8. Здесь можно наверное чуть проще сделать, как то вот так

   user = UserServ.get_obj_by_name(user_info.email, True)
   if not user:
   try:
       ....

9. Здесь интересный момент, вы создаете сессию к БД, но я не нашел кода где бы она закрывалась. Посмотрите, как это сделано например во Flask-SQLAlchemy интересно почему Вы отказались от ее использования ?
10. Здесь вы полагаетесь только на user-agent и в принципе это может приводить к тому что 2 пользователя которые могут иметь одинаковый UA - будут влиять друг на друга. Т.е. по сути у Вас происходит троттлинг определенного ua. Возможно ситуацию немного можно улучшить если вы будете учитывать еще например IP адрес (тоже не идеально, но уже лучше, можно ради интереса посмотреть на библиотечку https://flask-limiter.readthedocs.io/en/stable/ , использовать необязательно конечно, но можете глянуть как там устроено).

[ZOMini]

1. Да, c Flask-Migrate все попроще пошло, спасибо.
2. Ok
3. Ok, прогнал.
4. Ok, спасибо, для аннотаций самое то.
5. Понял, но всетаки это чистая CRUD ручка, да и метод RoleServ.role_crud() только разроучивает типы запросов, а далее уже для каждого типа свой метод, но понял, задумался. p.s. Подумал. Это все Джанго виноват:), там так учат.
6. Ok, get тоже переделал.
7. Ok
8. Ok, тут как обычно, задумал одно, сделал другое, переключился дальше, а получилось что то странное.
9. Ok, добавил в app.py. Когда начал - мне не понравилось что Flask-SQLAlchemy почему-то не "тянул" аннотации/докстринги (мб в VCS трабл, или во мне:) ). А потом уже не захотелось переделывать.
10. Ok, сделал user_agent+ip, библиотека интересная/гибкая, буду иметь ее ввиду в будущем.

[BigDeepBlue]

Молодцы, быстренько поправили, принято :) Спасибо большое ребята за обратную связь, за Ваши комментарии по пунктам, это важно. Еще неплохо в командной разработке пользоваться pull requests :), почему то у Вас не было ни одного PR.

[ZOMini]

И Вам спасибо! Чет ребята в академ собрались, так что без PR в этот раз.