-
Notifications
You must be signed in to change notification settings - Fork 250
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
feat(cmd)_: status-backend
#5847
Conversation
Jenkins BuildsClick to see older builds (98)
|
ac82091
to
5462930
Compare
@@ -99,6 +99,7 @@ func initializeApplication(requestJSON string) string { | |||
return string(data) | |||
} | |||
|
|||
// Deprecated: Use InitializeApplication instead. | |||
func OpenAccounts(datadir string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing new here, it was deprecated already. It's just done for the private func:
Lines 107 to 109 in b3143ce
// DEPRECATED: use InitializeApplication | |
// openAccounts opens database and returns accounts list. | |
func openAccounts(datadir string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have gathered the list of funcs manually, but it would be not simple to keep it up to date.
5462930
to
16a4a48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
I like we ended up with one server, CallRPC
endpoint is what I had in mind under term of "proxy", more or less.
91cd92a
to
1d8e0b2
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## develop #5847 +/- ##
==========================================
Coverage ? 46.06%
==========================================
Files ? 884
Lines ? 157156
Branches ? 0
==========================================
Hits ? 72397
Misses ? 76447
Partials ? 8312
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the great work, a big step forward for a fast development experience although there still need some changes for mobile frontend. 🚀
} | ||
|
||
if isDeprecated { | ||
isDeprecated = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should support deprecated by default and maybe add a switch to disable it. Consider that the mobile client will use the server one day. That would be a really cool experience for mobile developers as they won't need to endure the long wait to re-build with status-go with each modification to status-go to debug or do something else ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense indeed, will do 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a README.md with the Usage you describe in your PR description?
fix(statusd)_: manually serve
1ce9b15
to
2fa34eb
Compare
cmd/statusd/server/parse-api/main.go
Outdated
|
||
// Function to check if a comment indicates a deprecated function | ||
func isDeprecatedComment(line string) bool { | ||
return strings.Contains(line, "// Deprecated:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sometimes it's // DEPRECATED
or @deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. It will work in this file for all public methods now, but will make it a bit smarter 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simple case-insensitive first comment word contains "deprecated" should do, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 2ba2613
Checking it like this:
deprecatedRegex = regexp.MustCompile(`(?i)//\s*Deprecated`) |
Which matches https://go.dev/wiki/Deprecated.
Not bothering about @deprecated
form, as it's not conventional in Go. Anyway, it's only used in wallet/api.go
, and is not parsed by this program.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great tool!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
power/simplicity ratio off the charts, awesome job!
Closes #5828
Description
This PR introduces a new tool for debugging and testing
status-go
.In contrast to existing
statusd
andstatus-cli
, thestatus-backend
exposes full status-go API through HTTP.This allows to communicate with status-go through HTTP the same way as
status-desktop
andstatus-mobile
do, including: create account, restore account, login, logout, start messenger, start wallet and subscribe to status-go signals.Implementation
I decided to keep
statusd
as is. Reasoning here.Yet, when functional tests migrate to use
status-bacend
, we can remove the--seed-pharse
argument fromstatusd
. It was introduced specifically for it.@osmaczko I kept
signals_server
instatusd
for easier review of the changes in it.After this is merged, I will open a separate PR with just moving and renaming the files.
To continue the idea of a single server for both APIs:
I found a better solution. When using
status-backend
, you don't need to start geth's RPC server at all. You can accessservices/*
API throughCallRPC
endpoint, just the same as desktop/mobile do.Usage
Start the app with the address to listen to:
Access the exposed API with any HTTP client you prefer:
Simple flows
Create account and login
InitializeApplication
CreateAccountAndLogin
wakuext_startMessenger
wallet_startWallet
settings_getSettings
(temporary workaround, otherwise settings don't get saved into DB)Login into account
InitializeApplication
LoginAccount
wakuext_startMessenger
wallet_startWallet
Requests collection
Not to pollute this PR, a follow-up PR with some basic requests is here: