Skip to content

PR - Better handling of async_result (payment_done)#4153

Merged
rakanalh merged 2 commits into
raiden-network:developfrom
offerm:PR_async_results
May 29, 2019
Merged

PR - Better handling of async_result (payment_done)#4153
rakanalh merged 2 commits into
raiden-network:developfrom
offerm:PR_async_results

Conversation

@offerm
Copy link
Copy Markdown
Contributor

@offerm offerm commented May 28, 2019

the raiden_event_handler now returns to the payment rest layer an instance of EventPaymentSentFailed or EventPaymentSentSuccess. Better handling for error situation and allow the failure reason to be provided back to the caller.

@offerm offerm changed the title Better handling of async_result (payment_done) PR - Better handling of async_result (payment_done) May 28, 2019
@offerm offerm mentioned this pull request May 28, 2019
Copy link
Copy Markdown
Contributor

@rakanalh rakanalh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will approve once my comment is implemented.

Comment thread raiden/api/rest.py Outdated
return api_error(
errors="Payment couldn't be completed "
"(insufficient funds, no route to target or target offline).",
"(insufficient funds, no route to target or target offline). " + result.reason,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could change the three lines above to:

return api_error(
    errors=f"Payment couldn't be completed because: {result.reason}",

The message before the reason is not necessary anymore because in the places we emit the EventPaymentSentFailed and we provide the reason.

Offer Markovich added 2 commits May 29, 2019 09:09
the raiden_event_handler now returns to the payment rest layer an instance of EventPaymentSentFailed or EventPaymentSentSuccess. Better handling for error situation and allow the failure reason to be provided back to the caller.
Instead use the reason provided with EventPaymentSentFailed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2019

Codecov Report

Merging #4153 into develop will increase coverage by 0.07%.
The diff coverage is 40%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4153      +/-   ##
===========================================
+ Coverage    86.75%   86.83%   +0.07%     
===========================================
  Files          102      102              
  Lines        12935    12936       +1     
===========================================
+ Hits         11222    11233      +11     
+ Misses        1713     1703      -10
Impacted Files Coverage Δ
raiden/api/rest.py 86.41% <0%> (+0.26%) ⬆️
raiden/raiden_event_handler.py 96.59% <100%> (+0.37%) ⬆️
raiden/utils/runnable.py 88.23% <0%> (-2.95%) ⬇️
raiden/waiting.py 86.23% <0%> (-2.76%) ⬇️
raiden/network/proxies/secret_registry.py 84.48% <0%> (-2.59%) ⬇️
raiden/connection_manager.py 81.52% <0%> (-2.55%) ⬇️
raiden/api/v1/resources.py 98.91% <0%> (-1.09%) ⬇️
raiden/blockchain/events.py 99% <0%> (-1%) ⬇️
raiden/tasks.py 95.58% <0%> (-0.74%) ⬇️
raiden/blockchain_events_handler.py 99.47% <0%> (-0.53%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b144132...e1e0940. Read the comment docs.

@offerm offerm force-pushed the PR_async_results branch from 0af425c to e1e0940 Compare May 29, 2019 07:36
@offerm
Copy link
Copy Markdown
Contributor Author

offerm commented May 29, 2019

@rakanalh made the change. Thanks

@rakanalh rakanalh merged commit dc3ce0d into raiden-network:develop May 29, 2019
@rakanalh
Copy link
Copy Markdown
Contributor

Thanks @offerm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants