WIP Repl commands #144

lucasalcantara · 2018-06-05T14:04:42Z

Related issue #122

What has been done

Implemented commands in Spacemesh REPL
Changed abort signal from os/signal to go-prompt lib(now to abort the user needs to press Control + D)
Mocked node to implement the new commands
Added ENV variable

Commands added

create account
unlock account
lock account
account
transfer coins
setup
restart node
set
echo

lucasalcantara · 2018-06-05T14:07:25Z

@y0sher please, take a look at this new PR.

y0sher · 2018-06-05T14:33:15Z

hi please gofmt and golint and push again before I review :)

lucasalcantara · 2018-06-05T15:24:53Z

@y0sher sorry for the problems with gofmt and golint. Now the problem is inappropriate ioctl for device and apparently the lib has an issue related with it but is happening just when we execute the tests. So I think you can review it and I've subscribed at the issue to keep tracking any news.

y0sher

Please see my comments, I'm changing title to WIP.

y0sher · 2018-06-06T07:44:15Z

ENV

@@ -0,0 +1,2 @@
+DataPath=
+NetworkID=


I meant these should be exposed to the repl from the node's config. they are already presented there

y0sher · 2018-06-06T07:46:35Z

p2p/localnode.go

+	SetVariables(params, flags []string) error
+	Restart(params, flags []string) error
+	NeedRestartNode(params, flags []string) bool
+	Setup(allocation string) error


I'd rather you create an interface inside repl that has these methods and then in the future we could use LocalNode as this interface if we implement them.

y0sher · 2018-06-06T07:47:21Z

p2p/localnodeimpl.go

+// Setup setup POST.
+func (n *localNodeImp) Setup(allocation string) error {
+	return nil
+}


As I said above, please take these out. you can name it a REPLClient interface

y0sher · 2018-06-06T07:48:18Z

repl/variables.go

+}
+
+func echo(key string) {
+	fmt.Println(os.Getenv(key))


You should implement normal echo that replaces ENV variables with the stored env key.

y0sher · 2018-06-06T08:17:33Z

repl/variables.go

+	"strings"
+)
+
+func setVariables(path string) error {


Instead of reading from a file the node (or interface that will represent a node in the future) should expose a function that return the variables that are exported and their values.

y0sher · 2018-06-06T08:21:57Z

repl/repl.go

+}
+
+// StartRepl starts REPL.
+func StartRepl(node p2p.LocalNode, envPath string) {


Replace node with an interface that holds the needed functions and that LocalNode can implement.

y0sher

Good job, please check my comments.

y0sher · 2018-06-09T09:01:45Z

repl/repl.go

 	if err != nil {
-		r.node.Debug(err.Error())
+		log.Debug(err.Error())
 	} else {


In go a good pattern is,

if err != nil { log.Error(err) retutn } r.setup()

Instead of using if/else just return inside the error condition

y0sher · 2018-06-09T09:06:54Z

repl/repl.go

+	err := r.client.Unlock(passphrase)
+	if err != nil {
+		log.Debug(err.Error())
+	} else {


Again , in idomatic go

if err != nil { log.Debug(err) return } // The code that was on the else

Note: this command refers to rest of the places you do the same.

Please check https://golang.org/doc/effective_go.html#errors

y0sher · 2018-06-09T09:36:07Z

repl/variables.go

+
+func setVariables(values map[string]string) error {
+	for key, value := range values {
+		err := os.Setenv(key, value)


By environment variables we meant variables contained inside the node running and not the OS level. This might be problematic if one runs more than one node on a machine and the variables clash

y0sher · 2018-06-09T09:40:20Z

repl/variables.go

+				value = strings.TrimSpace(fields[1])
+			}
+
+			err := os.Setenv(key, value)


Setting variables shouldn't happen from the echo command. Instead you should create a function that translate variables to their values (recognize variables starting with $. Then every command could use ENV variables.
Setting a variable should be another command (set/export) and should be contained inside the node software.

sorry if this is a silly question but what is ENV variable?

Environment variables are variables variables KEY/VALUE pairs that are exposed widely in operating systems so software can read and use them.
We want to simulate this feature inside the spacemesh node without affecting the actual operating system environment variables.
The repl should be able read such variables from the node. (As NetworkID and DataFilePath for example)

so let's see if I understood right. All that I have to do is get the variables that the user puts on console (e.g. $NetworkID), ask the node the variable and print the value on the console, right? there is no need for a method like setVariables like I created.

For now you can start with this. in the future we might want to enable users to set variables through the console aswell.

…into repl-commands

lucasalcantara · 2018-06-21T16:14:03Z

@y0sher, should I do any change or is it ok now?

y0sher · 2018-06-22T14:50:55Z

Great job @lucasalcantara , please write unit tests before we can merge it.

lucasalcantara · 2018-06-26T13:51:00Z

@y0sher I don't know if I can add unit test for it. Since the lib doesn't return the messages that it receives. So, if you have any idea how I can do it I will be glad of try.

antonlerner · 2018-12-18T14:59:56Z

Closed due to inactivity

antonlerner · 2019-01-30T14:54:13Z

Thanks for contributing this code! we are about to use it as our first CLI wallet!

antonlerner · 2019-02-10T14:31:42Z

Merged opied code int CLIWallet project

lucasalcantara added 9 commits June 4, 2018 00:02

structuring repl

99efb7b

Adding messages flow

ebe6221

refactoring messages

f3faead

adding ENV variable echoing mechanism and doing small refactoring

a74ecfa

mocking node methods

9a66e84

refactoring prompt

16b2e26

changing App stop signal to REPL stop signal

9cacb8d

resolving conflict

17811b9

Merge branch 'develop' into repl-commands

4cb0fc9

lucasalcantara added 2 commits June 5, 2018 16:44

fixing problems detected in golint

ffa975d

updating dependencies

8630f5d

fixing panic on unit test

db23d15

y0sher suggested changes Jun 6, 2018

View reviewed changes

y0sher changed the title ~~Repl commands~~ WIP Repl commands Jun 6, 2018

lucasalcantara added 2 commits June 6, 2018 18:31

creating repl client interface and struct to mocking client

69f1667

exposing config variables and fixing bug of repl not running

6ccfadd

y0sher suggested changes Jun 9, 2018

View reviewed changes

lucasalcantara added 3 commits June 14, 2018 10:12

Merge branch 'develop' of https://github.com/spacemeshos/go-spacemesh …

840bbc9

…into repl-commands

applying error best practice and getting ENV variables through the node

97dee97

gofmt

f0217bc

antonlerner closed this Dec 18, 2018

antonlerner reopened this Jan 30, 2019

antonlerner closed this Feb 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Repl commands #144

WIP Repl commands #144

lucasalcantara commented Jun 5, 2018 •

edited

Loading

lucasalcantara commented Jun 5, 2018

y0sher commented Jun 5, 2018

lucasalcantara commented Jun 5, 2018 •

edited

Loading

y0sher left a comment

y0sher Jun 6, 2018

y0sher Jun 6, 2018

y0sher Jun 6, 2018

y0sher Jun 6, 2018

y0sher Jun 6, 2018

y0sher Jun 6, 2018

y0sher left a comment

y0sher Jun 9, 2018

y0sher Jun 9, 2018

y0sher Jun 9, 2018

y0sher Jun 9, 2018

lucasalcantara Jun 10, 2018

y0sher Jun 11, 2018

lucasalcantara Jun 11, 2018 •

edited

Loading

y0sher Jun 13, 2018

lucasalcantara commented Jun 21, 2018

y0sher commented Jun 22, 2018

lucasalcantara commented Jun 26, 2018

antonlerner commented Dec 18, 2018

antonlerner commented Jan 30, 2019

antonlerner commented Feb 10, 2019

		@@ -0,0 +1,2 @@
		DataPath=
		NetworkID=

WIP Repl commands #144

WIP Repl commands #144

Conversation

lucasalcantara commented Jun 5, 2018 • edited Loading

What has been done

Commands added

lucasalcantara commented Jun 5, 2018

y0sher commented Jun 5, 2018

lucasalcantara commented Jun 5, 2018 • edited Loading

y0sher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

y0sher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasalcantara Jun 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasalcantara commented Jun 21, 2018

y0sher commented Jun 22, 2018

lucasalcantara commented Jun 26, 2018

antonlerner commented Dec 18, 2018

antonlerner commented Jan 30, 2019

antonlerner commented Feb 10, 2019

lucasalcantara commented Jun 5, 2018 •

edited

Loading

lucasalcantara commented Jun 5, 2018 •

edited

Loading

lucasalcantara Jun 11, 2018 •

edited

Loading