cmd: validate repo/api file and print nicer error message #3219

Kubuxu · 2016-09-13T13:54:40Z

Resolves #3191
License: MIT
Signed-off-by: Jakub Sztandera kubuxu@protonmail.ch

whyrusleeping · 2016-09-13T14:18:55Z

cmd/ipfs/main.go

-	if apiAddrStr == "" {
-		var err error
-		if apiAddrStr, err = fsrepo.APIAddr(repoPath); err != nil {
+	efmt := fmt.Sprintf("failed to parse %[1]s/api file.\n\terror: %%s\n"+


lets make this a multiline string constant somewhere

Kubuxu · 2016-09-13T16:58:57Z

Updated

Kubuxu · 2016-09-13T17:06:11Z

Updated once more.

whyrusleeping · 2016-09-13T17:08:26Z

cmd/ipfs/main.go

+	error: %[2]s
+If there is no daemon running, it is safe to delete it.
+You can do it with:
+	ps aux | grep ipfs # check there is no ipfs daemon


Not valid advice for windows... not sure what the best way to do this is.

cc @jbenet @lgierth

On Windows that would be tasklist | findstr ipfs, alternatively you can refer people to the Task Manager.

thanks @djdv, I think something like this works:

If you're sure go-ipfs isn't running, you can just delete it. Otherwise check: ps aux | grep ipfs # on Linux and OSX tasklist | findstr ipfs # on Windows

We can also generate this message in an init function where we can do a switch on runtime.GOOS

whyrusleeping · 2016-09-13T17:37:50Z

repo/fsrepo/fsrepo.go

 	}

 	s := string(buf[:n])
 	s = strings.TrimSpace(s)
-	return s, nil
+	m, err := ma.NewMultiaddr(s)


should just return ma.NewMultiaddr(s) here

whyrusleeping · 2016-09-30T22:22:29Z

@Kubuxu status update?

Kubuxu · 2016-09-30T22:27:19Z

I remember looking for a lib that we have already imported that included helpers for checking OSes, and then I did something else.

Kubuxu · 2016-10-03T16:11:37Z

Updated

whyrusleeping

A couple comments, then LGTM

whyrusleeping · 2016-10-04T00:24:37Z

cmd/ipfs/main.go

+			return nil, fmt.Errorf(apiErrorFmt, repoPath, err.Error())
+		}
+	}
+	if len(addr.Protocols()) == 0 {


Will this ever happen from a passed in apiAddrStr? We don't want to accidentally tell the user about an api file if theyre manually specifying the address

This happens when for some reason the API File is empty.

Right, But theres no possible way we can end up with that scenario if the user passes in an api addr with --api right?

No idea that is why I added check in 7512eb9

whyrusleeping · 2016-10-04T01:28:34Z

cmd/ipfs/main.go

@@ -32,7 +25,14 @@ import (
 	repo "github.com/ipfs/go-ipfs/repo"
 	config "github.com/ipfs/go-ipfs/repo/config"
 	fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo"
+
+	logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
+	"gx/ipfs/QmXuBJ7DR6k3rmUEKtvVMhwjmXDuJgXXPUt4LQXKBMsU93/go-os-helper"


since the package name and the directory name arent the same, lets add osh to name this import.

Kubuxu · 2016-10-04T16:39:43Z

Last commit should address it.

Kubuxu · 2016-10-04T20:14:32Z

package.json

+      "author": "Kubuxu",
+      "hash": "QmXuBJ7DR6k3rmUEKtvVMhwjmXDuJgXXPUt4LQXKBMsU93",
+      "name": "go-os-helper",
+      "version": "0.0.0"


oh, look at that

what am i looking at?

Oh, ok, not it is good. Just a second ago GH wasn't showing the bracket that is bellow this line.

whyrusleeping · 2016-10-28T05:51:21Z

@Kubuxu wanna rebase this and make sure the tests pass?

Kubuxu · 2016-11-02T17:04:48Z

Rebased.

Kubuxu · 2016-11-04T11:25:17Z

Recreated it from ground 0 (easier than resolving conflicts in import section).

License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>

Kubuxu · 2016-11-21T18:27:26Z

@whyrusleeping should be good for merge right now

Kubuxu added status/in-progress In progress need/review Needs a review and removed status/in-progress In progress labels Sep 13, 2016

whyrusleeping reviewed Sep 13, 2016
View reviewed changes

Kubuxu force-pushed the fix/api-file-bad branch from 385b9cf to 7c8a43d Compare September 13, 2016 16:58

Kubuxu force-pushed the fix/api-file-bad branch from 7c8a43d to 928b832 Compare September 13, 2016 17:06

whyrusleeping reviewed Sep 13, 2016
View reviewed changes

Kubuxu force-pushed the fix/api-file-bad branch from 928b832 to b1805a7 Compare September 13, 2016 17:10

whyrusleeping reviewed Sep 13, 2016
View reviewed changes

Kubuxu added need/author-input Needs input from the original author and removed need/review Needs a review labels Sep 30, 2016

Kubuxu self-assigned this Sep 30, 2016

Kubuxu force-pushed the fix/api-file-bad branch 2 times, most recently from a38e5cd to c4542cc Compare October 3, 2016 15:55

Kubuxu added need/review Needs a review and removed need/author-input Needs input from the original author labels Oct 3, 2016

Kubuxu removed their assignment Oct 3, 2016

whyrusleeping requested changes Oct 4, 2016

View reviewed changes

Kubuxu force-pushed the fix/api-file-bad branch from 82dde0d to 60195f0 Compare October 4, 2016 16:39

Kubuxu force-pushed the fix/api-file-bad branch 2 times, most recently from 3b89cf4 to 7512eb9 Compare October 4, 2016 16:42

Kubuxu commented Oct 4, 2016

View reviewed changes

Kubuxu self-assigned this Oct 28, 2016

Kubuxu removed the need/review Needs a review label Oct 28, 2016

Kubuxu force-pushed the fix/api-file-bad branch from 7512eb9 to 3199047 Compare November 2, 2016 17:03

Kubuxu force-pushed the fix/api-file-bad branch 3 times, most recently from 5865975 to 6068da7 Compare November 4, 2016 11:24

Kubuxu assigned whyrusleeping and unassigned Kubuxu Nov 4, 2016

whyrusleeping approved these changes Nov 21, 2016

View reviewed changes

Kubuxu force-pushed the fix/api-file-bad branch from 6068da7 to 022846e Compare November 21, 2016 18:18

cmd: validate repo/api file and print nicer error messag

1e3b4ae

License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>

Kubuxu force-pushed the fix/api-file-bad branch from 022846e to 1e3b4ae Compare November 21, 2016 18:20

Kubuxu added the RFM label Nov 21, 2016

whyrusleeping merged commit 9094cc8 into master Nov 21, 2016

whyrusleeping deleted the fix/api-file-bad branch November 21, 2016 18:31

ghost mentioned this pull request Dec 23, 2016

Code Review v0.4.4...v0.4.5 #3534

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd: validate repo/api file and print nicer error message #3219

cmd: validate repo/api file and print nicer error message #3219

Kubuxu commented Sep 13, 2016 •

edited

Loading

whyrusleeping Sep 13, 2016

Kubuxu commented Sep 13, 2016

Kubuxu commented Sep 13, 2016

whyrusleeping Sep 13, 2016

djdv Sep 18, 2016 •

edited

Loading

ghost Sep 18, 2016

whyrusleeping Sep 19, 2016

whyrusleeping Sep 13, 2016

whyrusleeping commented Sep 30, 2016

Kubuxu commented Sep 30, 2016

Kubuxu commented Oct 3, 2016

whyrusleeping left a comment

whyrusleeping Oct 4, 2016

Kubuxu Oct 4, 2016

whyrusleeping Oct 4, 2016

Kubuxu Oct 4, 2016

whyrusleeping Oct 4, 2016

Kubuxu commented Oct 4, 2016

Kubuxu Oct 4, 2016

whyrusleeping Oct 4, 2016

Kubuxu Oct 4, 2016

whyrusleeping commented Oct 28, 2016

Kubuxu commented Nov 2, 2016

Kubuxu commented Nov 4, 2016

Kubuxu commented Nov 21, 2016

cmd: validate repo/api file and print nicer error message #3219

cmd: validate repo/api file and print nicer error message #3219

Conversation

Kubuxu commented Sep 13, 2016 • edited Loading

Choose a reason for hiding this comment

Kubuxu commented Sep 13, 2016

Kubuxu commented Sep 13, 2016

Choose a reason for hiding this comment

djdv Sep 18, 2016 • edited Loading

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

whyrusleeping commented Sep 30, 2016

Kubuxu commented Sep 30, 2016

Kubuxu commented Oct 3, 2016

whyrusleeping 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

Kubuxu commented Oct 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whyrusleeping commented Oct 28, 2016

Kubuxu commented Nov 2, 2016

Kubuxu commented Nov 4, 2016

Kubuxu commented Nov 21, 2016

Kubuxu commented Sep 13, 2016 •

edited

Loading

djdv Sep 18, 2016 •

edited

Loading