Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace zerorpc server with REST API webserver #526

Merged
merged 94 commits into from Dec 21, 2019

Conversation

@LefterisJP
Copy link
Contributor

LefterisJP commented Oct 20, 2019

This PR closes #404

It also does the following:

  • Completely remove ZeroRPC and all its python and npm dependencies
  • Consolidates the API by restructuring some calls, and removing others. For example:
    • set_main_currency call is removed since the same can be achieved by set_settings({'main_currency': 'XXX'})
    • Almost all calls are now wrapped in the {'result': XXX, 'message': 'may have error message here'} object for consistency
  • Adds a new documentation page for the REST API
@LefterisJP

This comment has been minimized.

Copy link
Contributor Author

LefterisJP commented Oct 20, 2019

Hey @kelsos this is the WIP/Draft PR for replacing zerorpc with a normal webserver.

For the rotkehlchen_service.ts how can all the client.invoke() calls be changed to be http queries (GET, PUT, DELETE e.t.c.) on endpoints ?https://github.com/rotkehlchenio/rotkehlchen/pull/526/files#diff-5d937a0f461d44838215b9664e4ab53bR34

Also got any other remarks or comments on this ongoing work?

@kelsos

This comment has been minimized.

Copy link
Collaborator

kelsos commented Oct 20, 2019

I will check as soon as I am back to Berlin. Is that OK?

@LefterisJP LefterisJP force-pushed the rotki:master branch from c55a912 to 6f9d300 Oct 22, 2019
@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch from 939506c to c32d490 Oct 25, 2019
@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch 2 times, most recently from f4fdc96 to 28cf8fd Nov 8, 2019
@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch from cc8db63 to a16e970 Nov 18, 2019
@LefterisJP LefterisJP force-pushed the rotki:master branch 3 times, most recently from 3b27dc4 to 74a9d11 Dec 1, 2019
@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch 3 times, most recently from 5ae2b0b to fb2bdaa Dec 1, 2019
@LefterisJP LefterisJP changed the base branch from master to develop Dec 12, 2019
@LefterisJP

This comment has been minimized.

Copy link
Contributor Author

LefterisJP commented Dec 12, 2019

The default branch for development of big features has been changed to develop. I updated the PR's base branch to reflect this.

@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch from 05c3a8f to cc98872 Dec 14, 2019
LefterisJP added 13 commits Aug 6, 2019
It is not needed. The same thing can be achieved via
set_settings({'main_currency': 'WHATEVER'})
- query_task_result renamed to query_task_outcome
- a proper response structure that also detects and shows pending
  tasks is now utilized
- changes in the front-end code to properly query the new endpoint
In the old API this used to deal only with external trades but there
is no more reason for this to happen as users of the API can and
probably will deal with all trade locations in the future.

As such now the front-end's external trades table must query for
specific `location="external"` trades
LefterisJP added 5 commits Dec 12, 2019
@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch from cc98872 to 87f4b7c Dec 14, 2019
@LefterisJP

This comment has been minimized.

Copy link
Contributor Author

LefterisJP commented Dec 14, 2019

Just rebased on top of develop and all the vue.js work.

LefterisJP added 4 commits Dec 14, 2019
@LefterisJP LefterisJP force-pushed the LefterisJP:workon_404 branch from 5917ed0 to 8fd3810 Dec 16, 2019
LefterisJP added 9 commits Dec 16, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 19, 2019

Codecov Report

Merging #526 into develop will increase coverage by 3.57%.
The diff coverage is 91.27%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #526      +/-   ##
===========================================
+ Coverage    77.54%   81.12%   +3.57%     
===========================================
  Files           53       69      +16     
  Lines         5143     7998    +2855     
  Branches       751     1156     +405     
===========================================
+ Hits          3988     6488    +2500     
- Misses         952     1199     +247     
- Partials       203      311     +108
Impacted Files Coverage Δ
rotkehlchen/history.py 91.48% <ø> (ø)
rotkehlchen/typing.py 91.87% <ø> (-0.09%) ⬇️
rotkehlchen/exchanges/bittrex.py 79.02% <ø> (ø)
rotkehlchen/exchanges/kraken.py 66.28% <0%> (ø)
rotkehlchen/server.py 0% <0%> (-59.34%) ⬇️
rotkehlchen/args.py 0% <0%> (ø)
rotkehlchen/serialization/deserialize.py 84.77% <100%> (+6.07%) ⬆️
rotkehlchen/exchanges/poloniex.py 76.82% <100%> (ø)
rotkehlchen/constants/assets.py 100% <100%> (ø) ⬆️
rotkehlchen/data_handler.py 91.66% <100%> (+9.68%) ⬆️
... and 53 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 6339a22...97cf674. Read the comment docs.

Fix #519.

This should no longer happen. The very first upgrade of the DB turned
all address entries into checksummed entries. Since then we make sure
to always add addresses as checksummed in the DB and the API also
makes sure that all address input is checksummed.

So this commit closes the chapter by warning the user if a
non-checksummed eth account is found in his DB. But the only way this
happens is if the user manually edits it.
@kelsos kelsos mentioned this pull request Dec 21, 2019
@LefterisJP LefterisJP marked this pull request as ready for review Dec 21, 2019
@LefterisJP

This comment has been minimized.

Copy link
Contributor Author

LefterisJP commented Dec 21, 2019

Merging this one and will rebase #573 on top of develop.

@LefterisJP LefterisJP merged commit 3b0a7bc into rotki:develop Dec 21, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@LefterisJP LefterisJP deleted the LefterisJP:workon_404 branch Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.