Добавлен базовый api для frontend #19
Assignees
alex (Aleksey Lobanov)
Clear assignees
alex (Aleksey Lobanov)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: alex/django-todo#19
Reference in New Issue
Block a user
Довольно много пожеланий по стилю, которые, как мне кажется, стоит поправить.
Также нет поддержки ToDoItem, только списки. Эта поддержка нужна хоть какая-то, даже если методы будут просто заглушками, чтобы GUI можно было писать, опираясь на этот интерфейс
Такие импорты, в целом, плохая практика, особенно, на уровне не конечных модулей.
Лучше
import jsonОбычно, импорты принято сортировать по порядку: сначала стандартные и системные, потом библиотеки внешние, потом библиотеки внутренние потом какие-то проектные модули.
Лучше в принципе не использовать изменяемые аргументы как аргументы по умолчанию. Но если очень хочется, то можно использовать https://stackoverflow.com/a/52022425/1551213
Мне кажется, что слишком хитрая штука.
Для работы с url'ами следует использовать urljoin
Выбор имён мне кажется странным.
У пользователя есть метод delete, который на самом деле удаляет какой-то список этого пользователя? Это надо как-то улучшить, мне кажется
Вместо такого переопределения, мне кажется, лучше использовать
requests.get(json={"x":42}), а он уже сам поставит правильные заголовкиЕщё раз напишу, что использование кодов возврата за пределами этого модуля будет плохой идеей, как мне кажется
Использование методов с двойными подчёркиваниями является плохой практикой, т.к. обычно их для себя резервирует Python. Если это закрытый метод, то лучше использовать одинарное нижнее:
_do_somthingЭто точно нельзя хардкодить. Проще всего получать в конструкторе.
В целом, токены JWT можно считать долгоживущими, поэтому их лучше получать только один раз, а потом
А pre-commit хук точно отработал?
Модуль весьма мал, плюс не то чтобы нам требовалось что-то большое от json, однако если поддерживать такой стиль по всему проекту то почему бы и исправить.
Сохранилось при переходе от urllib к requests
Ну искать какой-то специальный класс неизменяемого словаря (его именование уже об этом говорит), в принципе не выходящего за пределы своего модуля для одного аргумента не хотелось.
Не попалась по названию (при поиске аналога os.path.join), учтем
Выбор имен на данный момент целиком и полностью соответствует перечню запросов в swagger-е, вместе с развитием понятия что там реализовано будет и развитие документации-названий по тому что реализовано в API.
Про POST и остальные не учел. Добавлять их[коды] тоже не хотелось бы, но показать юзеру почему не проходит его запрос без залезания в код api тоже надо бы. Исключения тут уместными не казались, тк операция может не пройти именно на стороне сервера, но для лаконичности наверно придется общаться через них.
ок
Принимать в конструкторе токен звучит неплохо. А разве мы генерируем новые на каждый запрос? Проблемы с авторизацией это уже за пределами первой версии api.
Да, привычка выделять, стоит использовать одинарные
Должно было быть в конструкторе
Токены нигде не сохраняются, но каждый запрос этот токен требует. Значит генерируем новый токен на каждый запрос
Добавил исправления стиля, плюс базовое понимание того что происходит в запросах. Дальше уже пишем "удобные" методы работы со структурами данных, когда будет лучше понятна их структура
Можно уточнить у коллег, но мне кажется, что текущий интерфейс требует изменений.
Если подумать, каким бы был клиент, если бы API уже было готово и было бы большим?
Мне кажется, что это сущности, с которыми мы работаем ApiList, ApiToDoItem (и другие, если бы они были) которые бы могли удаляться/создаваться/обновляться.
При этом они были бы как-то связаны непосредственно с настройками API (модуль-синглтон или отдельный класс), где под настройкой будет подразумеваться работа с транспортом HTTPS, json и JWT токенами.
Тогда эти сущности можно будет без изменений использовать на фронте или с минимальными изменениями.
Также удастся использовать мощную систему типов + возможно, typing
Я придерживаюсь позиции, что лучше нормальные имена всегда, когда возможно, чем документация.
Также принято, что комментарии лучше использовать для ответа на вопрос "Зачем?", а не на вопрос "Что происходит?".
Например, переименовать
def delete(self, id):в
def delete_list(self, list_id)Заодно удастся избавиться от перекрытия имени
id, что тоже не очень хорошоТут мы говорим, что токен это словарь, т.е. как бы отказываемся от проверки типов. Типы нужны, чтобы их проверять. Я бы предложил принимать два токена: на доступ и на обновление (опционален). Тогда удастся использовать более жёсткие контракты без потери какой-либо гибкости
Это имя соответствует запросу из сваггера, более того то что делает этот запрос это все еще догадка
Предполагается что юзер не знает об апи и формате токенов и пользуется только теми токенами что оно ему предоставляет. Но немного больше логики - да, можно
Текущие запросы в апи - ниузкоуровневые, 1-1 запросы. Тогда добавлю систему классов для работы с более высокоуровневыми командами.
Pull request closed