Skip to content
This repository was archived by the owner on Feb 7, 2024. It is now read-only.

Add DefaultShell as required by #69 #106

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add DefaultShell as required by #69 #106

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
22e2ede
Add DefaultShell as required by #69
Jul 13, 2018
a48d7d7
use sync.Once to obtain default shell
Jul 13, 2018
ede2fd1
reuse NewLocalShell, add DefautlShell
Jul 28, 2018
921c4e1
Merge branch 'master' of https://github.com/ipfs/go-ipfs-api into def…
Jul 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions shell.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,42 @@ func NewShellWithClient(url string, c *gohttp.Client) *Shell {
}
}

// Get shell from environmental variables, default api path or gateway.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should spell out exactly how we pick the default shell.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think if I add some functions which respectively obtain new shells from environmental variables, default gateway and the api file. I think the order of shells to try in the function DefautlShell is sensible, as normally the environmental variables are explicitly set by the user and I have tried to use gateway as the the last resort.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

func DefaultShell() (*Shell, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to use a sync.Once and cache the default client. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will DefaultShell be called more than once? I think people will normally call it for one shot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see people calling DefaultShell().DoThing() quite often.

urls := make([]string, 1)
if ipfsAPI := os.Getenv("IPFS_API"); ipfsAPI != "" {
urls = append(urls, ipfsAPI)
}
ipfsPath := os.Getenv("IPFS_PATH")
if ipfsPath == "" {
if homePath, err := homedir.Dir(); err == nil {
ipfsPath = homePath
}
}
if ipfsPath != "" {
apifile := path.Join(ipfsPath, ".ipfs", "api")
if data, err := ioutil.ReadFile(apifile); err == nil {
url := strings.Trim(string(data), "\n\t ")
urls = append(urls, url)
}
}
urls = append(urls, "/ip4/127.0.0.1/tcp/5001", "https://ipfs.io")

// do not repeat encountered addresses
encountered := map[string]bool{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idiom is to just use map[string]struct{} and check for presence.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your advise. I will revise it to use map[string]struct{}.

for _, url := range urls {
if encountered[url] != true {
encountered[url] = true
sh := NewShell(url)
_, _, err := sh.Version()
if err == nil {
return sh, nil
}
}
}
return nil, errors.New("No default node is working")
}

func (s *Shell) SetTimeout(d time.Duration) {
s.httpcli.Timeout = d
}
Expand Down