ED-305: withdrawal repair tools #411
Conversation
…outes and to make sure of withdrawal succeeded
apps/ff_cth/src/ct_domain_config.erl
Outdated
| @@ -31,6 +34,11 @@ all(Revision) -> | |||
| #'Snapshot'{domain = Domain} = dmt_client:checkout(Revision), | |||
| Domain. | |||
|
|
|||
| -spec checkout_object(revision(), object_ref()) -> object() | no_return(). | |||
| checkout_object(Revision, ObjectRef) -> | |||
| #'Snapshot'{domain = Domain} = dmt_client:checkout(Revision), | |||
There was a problem hiding this comment.
В dmt_client теперь же есть оптимальный checkout_object.
| ] | ||
| ], | ||
| Result |
There was a problem hiding this comment.
Какое-то немного бессмысленное изменение, не?
There was a problem hiding this comment.
Плохо почистил всё после того как вставлял printf'ы, еще приберусь.
| @@ -761,6 +795,12 @@ do_process_transfer(adjustment, Withdrawal) -> | |||
| do_process_transfer(stop, _Withdrawal) -> | |||
| {undefined, []}. | |||
|
|
|||
| -spec process_repair(repair_scenario(), withdrawal_state()) -> process_result(). | |||
| process_repair({routing, {route_changed, Route}}, _Withdrawal) -> | |||
| {{set_timer, 0}, [{route_changed, Route}]}; | |||
There was a problem hiding this comment.
А зачем новый action? Чем это от continue отличается?
| -spec do_process_repair(repair_scenario(), withdrawal_state()) -> process_result(). | ||
| do_process_repair(Scenario, Withdrawal) -> | ||
| process_repair(Scenario, Withdrawal). |
There was a problem hiding this comment.
Нахожу эту функцию немного бессмысленной. 🤔 Зачем она тут?
There was a problem hiding this comment.
Это небольшой артефакт, который остался после того как я пробовал разные варианты запуска сценария починки. Сначала я эту функцию завел для единообразия с do_process_transfer, а потом оказалось что здесь делать нечего, но пока я это оставил. Выпилю, если она пустой останется.
| build_failure({route_not_found, Reason}, _Withdrawal) -> | ||
| #{code => Reason}; |
There was a problem hiding this comment.
Вообще выглядит немного странно: сценарий называется route_not_found, но при этом withdrawal просто завершится с произвольной ошибкой. Причём тут тогда route_not_found? 🤔
There was a problem hiding this comment.
Задумка была в том, что тот кто запустит восстановление по сценарию route_not_found напишет Reason, и это можно будет легко найти в логах, либо спустя время вспомнить что это был не просто "no_route_found", а что-то типа "Hand repair with no_route_found". Я упустил, что в этом случае в Reason'е может не оказаться слов про no_route_found (по умолчанию если инициатор поленится указать Reason, то будет дефолтное <<"no_route_found">>).
Тут два варианта, либо вообще Reason выпилить, либо вместо #{code => Reason} делать что-то типа #{code => <<"no_route_found, reason: ", Reason">>}. Вопрос в полезности этой фичи. Как считаешь?
There was a problem hiding this comment.
(подозреваю, что поле 'code' задумывалось как нехудожественное короткое обозначение ошибки, и значит не надо там писать никаких ризонов, короче наверное надо просто выпилить Reason и всё)
There was a problem hiding this comment.
Тут два варианта, либо вообще Reason выпилить, либо вместо
#{code => Reason}делать что-то типа#{code => <<"no_route_found, reason: ", Reason">>}. Вопрос в полезности этой фичи. Как считаешь?
Я думаю, что для такого сценарий должен по-другому называться, типа MakeFailed.
There was a problem hiding this comment.
надо просто выпилить Reason и всё)
Для текущего сценария определённо.
| @@ -188,8 +188,18 @@ process_call(CallArgs, _Machine, _, _Opts) -> | |||
|
|
|||
| -spec process_repair(ff_repair:scenario(), machine(), handler_args(), handler_opts()) -> | |||
| {ok, {repair_response(), result()}} | {error, repair_error()}. | |||
| process_repair({add_events, _} = Scenario, Machine, _Args, _Opts) -> | |||
| ff_repair:apply_scenario(ff_withdrawal, Machine, Scenario); | |||
There was a problem hiding this comment.
Кажется по задумке автора ff_repair ты просто процессор должен был добавить для этого сценария и передать его в ff_repair:apply_scenario/4.
| @@ -7,6 +7,8 @@ | |||
| -include_lib("damsel/include/dmsl_payment_processing_thrift.hrl"). | |||
| -include_lib("damsel/include/dmsl_withdrawals_provider_adapter_thrift.hrl"). | |||
|
|
|||
| -include_lib("fistful_proto/include/ff_proto_withdrawal_thrift.hrl"). | |||
There was a problem hiding this comment.
А зачем тут новый include? Чё-то не вижу сходу потребности.(как кстати и в предыдущем include)
| @@ -170,7 +173,7 @@ | |||
| -type invalid_withdrawal_status_error() :: | |||
| {invalid_withdrawal_status, status()}. | |||
|
|
|||
| -type action() :: sleep | continue | undefined. | |||
| -type action() :: sleep | continue | undefined | {set_timer, integer()}. | |||
There was a problem hiding this comment.
Тогда это изменение тоже можно убрать.
…tc (but there is problem - machine Ctx not cleared by repair_scenario and it running every process_timeout, need purge ctx in process_timeout ends)
…rk some imperative messy code
No description provided.