Skip to content

Commit

Permalink
Clarifying comments instead of TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
liamsi committed Feb 26, 2021
1 parent 2e9912c commit 4675af8
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,10 @@ func (n *Node) OnStart() error {
}
}
if n.embedIpfsNode {
// It is essential that we create a fresh instance of ipfs node on
// each start as internally the node gets only stopped once per instance.
// At least in ipfs 0.7.0; see:
// https://github.com/lazyledger/go-ipfs/blob/dd295e45608560d2ada7d7c8a30f1eef3f4019bb/core/builder.go#L48-L57
n.ipfsNode, err = createIpfsNode(n.config, n.areIfsPluginsAlreadyLoaded, n.Logger)
if err != nil {
return fmt.Errorf("failed to create IPFS node: %w", err)
Expand Down Expand Up @@ -1018,8 +1022,9 @@ func (n *Node) OnStop() {
}

if n.ipfsNode != nil {
// TODO(ismail): As far as I understand, the node gets shut down by cancelling the
// context that was passed into the node. But calling Close() seems cleaner and we don't
// Internally, the node gets shut down by cancelling the
// context that was passed into the node.
// Calling Close() seems cleaner and we don't
// need to keep a reference to the context around.
if err := n.ipfsNode.Close(); err != nil {
n.Logger.Error("ipfsNode.Close()", err)
Expand Down Expand Up @@ -1462,7 +1467,10 @@ func createIpfsNode(config *cfg.Config, arePluginsAlreadyLoaded bool, logger log
// Routing: libp2p.DHTClientOption,
Repo: repo,
}
// TODO: change to context.WithCancel if we want to use the context for lifecycle management
// Internally, ipfs decorates the context with a
// context.WithCancel. Which is then used for lifecycle management.
// We do not make use of this context and rely on calling
// Close() on the node instead
ctx := context.Background()
node, err := ipfscore.NewNode(ctx, nodeOptions)
if err != nil {
Expand Down

0 comments on commit 4675af8

Please sign in to comment.