в компанию, которая на входе дразнила тебя своими принципами чистого кода, а по факту тебе придется всю жизнь переписывать легаси и разбираться в чужих багах на проде
Ага, это я перепутал на какой стороне Вася написал код.
Сейчас попробую развернуть свою мысль про PUT. Если мы примем за факт то, что одна поездка может иметь только одну оценку, то ключом идемпотентности всего запроса можно считать order_id. Семантика метода PUT говорит нам о том, что ресурс должен быть изменен (или создан, если его еще не существует), что дает нам право рассчитывать на то, что повторный вызов этого метода не будет пытаться создавать новые ресурсы (задачи для саппорта). Согласно RFC7231 метод PUT является идемпотентным, а POST - нет.
Ок, я понимаю, что в Вашем примере Вася сделал идемпотентным вызов send_to_support, но по поводу остального мы никаких гарантий дать не можем. Т.е. я, например, не уверен, что вызванный дважды write_reasons_to_db не создаст две записи в БД.
Теперь представим, что мы изменили метод на PUT, и в соответствии с его семантикой давайте попробуем изменить код под капотом, для этого я предлагаю использовать в функциях работы с БД не INSERT, а UPSERT. Я уверен, что сейчас оно так и есть, но вот название функции add_rating наводит меня на мысль, что под капотом у нее что-то вроде INSERT. Если это не так - изменим ее название.
После того, как мы убедились, что все вставки в БД работают как UPSERT и при создании новой заявки в суппорт (что тоже лучше сделать как PUT) не создается дубликатов, мы можем считать, что код работает как обработчик PUT. Может быть в моем тексте много букв и кое-что уводит от того посыла, который я пытаюсь сюда вложить, поэтому я сейчас выражу его в пунктах:
давайте стараться не делать ретраи для POST, хотите ретраить - безопаснее это делать на бэкэнде (ну запуште сообщение в брокер и обрабатывайте, когда сможете, в Вашем примере пользователю все равно, когда это произойдет)
если мы сможем сделать идемпотентным все тело обработчика, то можно использовать метод, который в RFC7231 обозначен, как идемпотентный (тогда даже прокси сможет сделать вам ретрай)
если выполнение запроса отвалилось с ошибкой 5xx, то хорошо бы, чтобы мы не имели неконсистентные данные (хочется атомарности, откатите половину изменений, которые внесли)
Кроме того, если send_to_support поставить после вызова write_better_quality_reasons_to_db, то отказоустойчивость этого эндпоинта улучшится прямо на пустом месте за бесплатно.
Ну и все это пока не касается гонок, curl и прочих особенных случаев. Только безопасного повторного выполнения
Можно я немного позанудствую по поводу идемпотентности?
Я понимаю посыл, который в этой части вложен, понимаю, что там описана такая логика ретрая (ну может еще с отбоем по кол-ву ретраев): `repeat until PostSaveFeedback(stars, reasons, better_quality_reasons, order_id)` но представленное решение не до конца правильное. Истинной идемпотентности можно было добиться дополнительно изменив метод запроса /save-feedback с POST на PUT.
В таком случае не сломается логика, если по прошествии трех плохих ретраев обработчик вернет управление UI, а там пользователь сможет поменять ответ (ну кто его знает, бизнес-требования все время изменяются). В текущем решении мы рискуем получить разные данные для better_quality_reasons между вот этими вызовами: send_to_support(better_quality_reasons, support_task_id); write_better_quality_reasons_to_db(better_quality_reasons, order_id); ведь send_to_support отвалится с DuplicateTask
Однако, такое решение опять сломает обратную совместимость, ведь часть серверов еще может не поддерживать PUT, как Вася уже начнет выкатывать все это на фронт.
Справедливо.
Да я тоже. Эта секция была написана вообще, как симметричный ответ на один коммент в стиле "Россию в игнор ..."
Я ж даже пожалел, что написал об этом. Все равно не смог толком тему раскрыть
Ничего плохого
'''
в компанию, которая на входе дразнила тебя своими принципами чистого кода, а по факту тебе придется всю жизнь переписывать легаси и разбираться в чужих багах на проде
'''
Тут акцент на контрасте ожиданий и реальности
Ага, это я перепутал на какой стороне Вася написал код.
Сейчас попробую развернуть свою мысль про PUT. Если мы примем за факт то, что одна поездка может иметь только одну оценку, то ключом идемпотентности всего запроса можно считать order_id. Семантика метода PUT говорит нам о том, что ресурс должен быть изменен (или создан, если его еще не существует), что дает нам право рассчитывать на то, что повторный вызов этого метода не будет пытаться создавать новые ресурсы (задачи для саппорта). Согласно RFC7231 метод PUT является идемпотентным, а POST - нет.
Ок, я понимаю, что в Вашем примере Вася сделал идемпотентным вызов send_to_support, но по поводу остального мы никаких гарантий дать не можем. Т.е. я, например, не уверен, что вызванный дважды write_reasons_to_db не создаст две записи в БД.
Теперь представим, что мы изменили метод на PUT, и в соответствии с его семантикой давайте попробуем изменить код под капотом, для этого я предлагаю использовать в функциях работы с БД не INSERT, а UPSERT. Я уверен, что сейчас оно так и есть, но вот название функции add_rating наводит меня на мысль, что под капотом у нее что-то вроде INSERT. Если это не так - изменим ее название.
После того, как мы убедились, что все вставки в БД работают как UPSERT и при создании новой заявки в суппорт (что тоже лучше сделать как PUT) не создается дубликатов, мы можем считать, что код работает как обработчик PUT. Может быть в моем тексте много букв и кое-что уводит от того посыла, который я пытаюсь сюда вложить, поэтому я сейчас выражу его в пунктах:
давайте стараться не делать ретраи для POST, хотите ретраить - безопаснее это делать на бэкэнде (ну запуште сообщение в брокер и обрабатывайте, когда сможете, в Вашем примере пользователю все равно, когда это произойдет)
если мы сможем сделать идемпотентным все тело обработчика, то можно использовать метод, который в RFC7231 обозначен, как идемпотентный (тогда даже прокси сможет сделать вам ретрай)
если выполнение запроса отвалилось с ошибкой 5xx, то хорошо бы, чтобы мы не имели неконсистентные данные (хочется атомарности, откатите половину изменений, которые внесли)
Кроме того, если send_to_support поставить после вызова write_better_quality_reasons_to_db, то отказоустойчивость этого эндпоинта улучшится прямо на пустом месте за бесплатно.
Ну и все это пока не касается гонок, curl и прочих особенных случаев. Только безопасного повторного выполнения
Можно я немного позанудствую по поводу идемпотентности?
Я понимаю посыл, который в этой части вложен, понимаю, что там описана такая логика ретрая (ну может еще с отбоем по кол-ву ретраев):
`repeat until PostSaveFeedback(stars, reasons, better_quality_reasons, order_id)`
но представленное решение не до конца правильное. Истинной идемпотентности можно было добиться дополнительно изменив метод запроса /save-feedback с POST на PUT.
В таком случае не сломается логика, если по прошествии трех плохих ретраев обработчик вернет управление UI, а там пользователь сможет поменять ответ (ну кто его знает, бизнес-требования все время изменяются). В текущем решении мы рискуем получить разные данные для better_quality_reasons между вот этими вызовами:
send_to_support(better_quality_reasons, support_task_id);
write_better_quality_reasons_to_db(better_quality_reasons, order_id);
ведь send_to_support отвалится с DuplicateTask
Однако, такое решение опять сломает обратную совместимость, ведь часть серверов еще может не поддерживать PUT, как Вася уже начнет выкатывать все это на фронт.
Спасибо за дополнения. А подскажите, куда сохраняете request/response?
Ой, вот, чего я забыл. Спасибо