Skip to content

Handle RecoverableError in the python api's set_total_deposit#4938

Merged
rakanalh merged 4 commits into
raiden-network:developfrom
LefterisJP:workon_4937
Sep 24, 2019
Merged

Handle RecoverableError in the python api's set_total_deposit#4938
rakanalh merged 4 commits into
raiden-network:developfrom
LefterisJP:workon_4937

Conversation

@LefterisJP
Copy link
Copy Markdown
Contributor

Fix #4937

Description

Please, describe what this PR does in detail:

  • If it's a bug fix, detail the root cause of the bug and how this PR fixes it.
  • If it's a new feature, describe the feature this PR is introducing and design decisions.
  • If it's a refactoring, describe why it is necessary. What are its pros and cons in respect to the previous code, and other possible design choices.

PR review check list

Quality check list that cannot be automatically verified.

  • Safety
  • Code quality
    • Error conditions are handled
    • Exceptions are propagated to the correct parent greenlet
    • Exceptions are correctly classified as recoverable or unrecoverable
  • Compatibility
    • State changes are forward compatible
    • Transport messages are backwards and forward compatible
  • Commits
    • Have good messages
    • Squashed unecessary commits
  • If it's a bug fix:
    • Regression test for the bug
      • Properly covers the bug
      • If an integration test is used, it could not be written as a unit test
  • Documentation
    • A new CLI flag was introduced, is there documentation that explains usage?
  • Specs
    • If this is a protocol change, are the specs updated accordingly? If so, please link PR from the specs repo.
  • Is it a user facing feature/bug fix?
    • Changelog entry has been added

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 20, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@5a0109a). Click here to learn what that means.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #4938   +/-   ##
==========================================
  Coverage           ?   80.77%           
==========================================
  Files              ?      120           
  Lines              ?    14519           
  Branches           ?     2238           
==========================================
  Hits               ?    11728           
  Misses             ?     2135           
  Partials           ?      656
Impacted Files Coverage Δ
raiden/api/python.py 68.33% <80%> (ø)

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 5a0109a...2103b6d. Read the comment docs.

@LefterisJP
Copy link
Copy Markdown
Contributor Author

Oh man come on. Changed 2 lines to catch an exception. Wrote an entire new test which recreates a race (took a bit of time as we did not have any such tests in the rest api) to trigger the bug seen in the issue ... and still the coverage is decreased?!?!

Comment thread raiden/api/python.py Outdated
Comment thread raiden/tests/integration/api/test_restapi.py
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, thanks for taking this one.

@rakanalh rakanalh merged commit 69cbd22 into raiden-network:develop Sep 24, 2019
@LefterisJP LefterisJP deleted the workon_4937 branch September 24, 2019 09:46
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.

Raiden does not handle recoverable errors for channel open & deposit

2 participants