-
Notifications
You must be signed in to change notification settings - Fork 715
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
Implement client commands of Massa API endpoints - [merged] #1609
Comments
In GitLab by @AureliaDolo assigned to @Adolo |
In GitLab by @yvan-sraka added 5 commits
|
In GitLab by @yvan-sraka assigned to @yvan-sraka |
In GitLab by @yvan-sraka added 2 commits
|
In GitLab by @yvan-sraka added 1 commit
|
In GitLab by @yvan-sraka added 1 commit
|
In GitLab by @yvan-sraka marked this merge request as ready |
In GitLab by @yvan-sraka unassigned @Adolo |
In GitLab by @yvan-sraka |
In GitLab by @yvan-sraka changed title from {-Resolve "-}Implement client commands{-"-} to Implement client commands{+ of Massa API endpoints+} |
In GitLab by @AureliaDolo Commented on api-private/src/lib.rs line 174 Why ? |
In GitLab by @AureliaDolo Commented on rpc-client/src/cmds.rs line 201 We don't have confirmation from the node that the ip was unbanned, only that the node received the message ? |
In GitLab by @AureliaDolo Commented on rpc-client/src/cmds.rs line 209 Same here, we don't have confirmation that the ip is indeed banned |
In GitLab by @AureliaDolo Commented on rpc-client/src/cmds.rs line 221 Same issue with confirmation |
In GitLab by @yvan-sraka Commented on api-private/src/lib.rs line 174 Because I guess that now the semantic of the API call is to unban a list of IPs? I could also do a map and not stop on the first failure you're right. |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 201 So, hum ... you right I can change the output message to "Request of unbanning successfully sent"? And we could think about a new API call to get the list of banned IPs? |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 221 Maybe here we can check if (if that's the client that launched the |
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 201 I think that just confirming that the request was sent is OK for now |
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 221 the client can also perform regular pings/healthchecks... on a purpose-build API endpoint |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 221 that's true, we have to design that (in a follow-up issue ?) |
In GitLab by @yvan-sraka Commented on api-private/src/lib.rs line 174 changed this line in version 4 of the diff |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 201 changed this line in version 4 of the diff |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 209 changed this line in version 4 of the diff |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 221 changed this line in version 4 of the diff |
In GitLab by @yvan-sraka added 1 commit
|
In GitLab by @yvan-sraka added 1 commit
|
In GitLab by @yvan-sraka added 6 commits
|
In GitLab by @yvan-sraka marked this merge request as draft |
In GitLab by @damip Commented on .direnv/drv line 1 what's this ? |
In GitLab by @yvan-sraka Commented on .direnv/cache-pre320334.82155ff501c line 1 urgh |
In GitLab by @yvan-sraka Commented on .direnv/drv line 1 sorry I really lack of https://gitlab.com/massalabs/massa/-/merge_requests/164 |
In GitLab by @yvan-sraka Commented on .direnv/cache-pre320334.82155ff501c line 1 changed this line in version 9 of the diff |
In GitLab by @yvan-sraka Commented on .direnv/drv line 1 changed this line in version 9 of the diff |
In GitLab by @yvan-sraka added 1 commit
|
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 215 should be started detached, so that when we close the client, it doesn't close the node |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 Right! |
In GitLab by @damip Commented on rpc-client/src/main.rs line 71 Guidelines say: no leading uppercase letter, and no punctuation at the end for error messages. |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 I guess the right way of doing it is https://stackoverflow.com/a/53478775/2879157 |
In GitLab by @yvan-sraka Commented on rpc-client/src/main.rs line 71 Thanks, I was not aware of that! |
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 215 needs to be portable |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 grrr ... that's true, hmm ... |
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 215 yeah we can try. Ideally we need a mechanism to avoid starting 2 instances by mistake |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 Euhm like a |
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 215 Or a healthcheck API endpoint, I'm not sure... |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 So we authorize, on the same machine, running two nodes with different ports? or we try to forbid that? Maybe it's a good (soft) security protection to say that one machine == one node (unless we deploy several parallel Massa-based networks) |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 Do we postpone this study/discussion in a follow-up issue? |
In GitLab by @yvan-sraka Commented on rpc-client/src/main.rs line 71 changed this line in version 10 of the diff |
In GitLab by @yvan-sraka added 11 commits
|
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 215 yes, maybe remove this command for now from the client so that we can merge and do a clean followup to add it |
In GitLab by @damip Commented on rpc-client/src/cmds.rs line 221 yeah let's do it in a followup |
In GitLab by @damip There are multiple "Failed to serialized command output.". Let's correct the grammar and the formatting everywhere :) "failed to serialize command output" |
In GitLab by @yvan-sraka Commented on rpc-client/src/cmds.rs line 215 changed this line in version 11 of the diff |
In GitLab by @yvan-sraka added 6 commits
|
In GitLab by @damip approved this merge request |
In GitLab by @AureliaDolo
Merges 293-implement-client-commands -> dev
Closes #293
The text was updated successfully, but these errors were encountered: