Skip to content
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

Detect if IPFS is initialized in a more robust way, to resolve #744 #845

Closed
wants to merge 1 commit into from

Conversation

dapplion
Copy link
Contributor

🦅 Pull Request

Detect if IPFS is initialized in a more robust way, by calling

ipfs config show

which exits with 1 if IPFS is not initialized and with 0 if it is.

Fixes #744

🚨 Test instructions

Install IPFS using SNAP
Run aragon ipfs

The current version produces this output

  ⠼ Start IPFS
    → Starting IPFS at port: 5001
    Add local files
(node:16693) UnhandledPromiseRejectionWarning: Error: Command failed: /snap/bin/ipfs init
Error: ipfs configuration file already exists!
Reinitializing would overwrite your keys.


initializing IPFS node at /home/lion/snap/ipfs/1170/.ipfs

    at makeError (/home/lion/aragon-cli/packages/aragon-cli/node_modules/execa/index.js:174:9)
    at Promise.all.then.arr (/home/lion/aragon-cli/packages/aragon-cli/node_modules/execa/index.js:278:16)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:16693) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside   ✖ Start IPFS
    → Starting IPFS timed out:
    Add local files
✖ Starting IPFS timed out:

and with this PR

  ✔ Start IPFS
  ✔ Add local files
ℹ IPFS daemon is now running. Stopping this process will stop IPFS

✔️ PR Todo

  • Include links to related issues/PRs
  • Update unit tests for this change
  • Update the relevant documentation
  • Clear dependencies on other modules that have to be released before merging this

@dapplion dapplion requested a review from 0xGabi as a code owner October 27, 2019 22:39
@dapplion
Copy link
Contributor Author

Also, the go-ipfs team recommends to test if the IPFS daemon is running by checking the swarm addresses ipfs/kubo#5983. I believe running

ipfs swarm addrs

would be better and simpler than the current strategy:

  • checking if the IPFS port is busy, and
  • calling the API at the /id endpoint

@0xGabi Let me know if this change would be of interest

@0xGabi
Copy link
Contributor

0xGabi commented Oct 29, 2019

@dapplion seems like a nice to have feature right now. Would like to know what other think about it, @macor161 @0x6431346e

@macor161
Copy link
Contributor

Yes I think it's an improvement from what we had before. Thanks @dapplion !
I will test it to make sure everything works.

Copy link
Contributor

@macor161 macor161 left a comment

Choose a reason for hiding this comment

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

@dapplion await execa(ipfsBin, ['config', 'show']) seems to always fail on my laptop (Mac). So it always try to initialize even when it is already initialized. Would you know why?

Until the problem is resolved, I propose we postpone this PR.

@dapplion
Copy link
Contributor Author

dapplion commented Oct 30, 2019

@dapplion await execa(ipfsBin, ['config', 'show']) seems to always fail on my laptop (Mac). So it always try to initialize even when it is already initialized. Would you know why?

Until the problem is resolved, I propose we postpone this PR.

Could you provide the logs / error output when attempting to run it? What OS do you have?

Also, does running directly on a terminal ipfs config show produce a different outcome? If so please paste the output

@macor161
Copy link
Contributor

@dapplion :

{ Error: Command failed: /Users/mathew/.nvm/versions/node/v10.16.0/bin/ipfs config show
Error: api not running


    at makeError (/Users/mathew/workspace/aragon-forks/aragon-cli/packages/aragon-cli/node_modules/execa/index.js:174:9)
    at Promise.all.then.arr (/Users/mathew/workspace/aragon-forks/aragon-cli/packages/aragon-cli/node_modules/execa/index.js:278:16)
    at process._tickCallback (internal/process/next_tick.js:68:7)

@0xGabi 0xGabi changed the base branch from master to develop November 5, 2019 14:21
@0xGabi
Copy link
Contributor

0xGabi commented Nov 24, 2019

@0x6431346e what do you think of including this change on the 2nd iteration?

@dapplion
Copy link
Contributor Author

I want to add that aragon ipfs start never works on my linux.

lion@lion:~/$ aragon ipfs start
  ⠇ Start IPFS
    → Starting IPFS at port: 5001
    Add local files
(node:17349) UnhandledPromiseRejectionWarning: Error: Command failed: /snap/bin/ipfs init
Error: ipfs configuration file already exists!
Reinitializing would overwrite your keys.


initializing IPFS node at /home/lion/snap/ipfs/1170/.ipfs

    at makeError (/home/lion/.nvm/versions/node/v10.15.3/lib/node_modules/@aragon/cli/node_modules/execa/index.js:174:9)
    at Promise.all.then.arr (/home/lion/.nvm/versions/node/v10.15.3/lib/node_modules/@aragon/cli/node_modules/execa/index.js:278:16)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:17349) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (reje  ✖ Start IPFS
    → Starting IPFS timed out:
    Add local files
✖ Starting IPFS timed out:

@0xGabi 0xGabi mentioned this pull request Dec 2, 2019
4 tasks
@stale stale bot added the abandoned label Dec 24, 2019
@aragon aragon deleted a comment from stale bot Dec 27, 2019
@stale stale bot removed the abandoned label Dec 27, 2019
@0xGabi 0xGabi added the 📟 cli A change related with cli tools label Jan 23, 2020
@stale stale bot added the abandoned label Jan 26, 2020
@aragon aragon deleted a comment from stale bot Jan 29, 2020
@stale stale bot removed the abandoned label Jan 29, 2020
@dapplion
Copy link
Contributor Author

dapplion commented Feb 1, 2020

I would advise to switch to https://github.com/ipfs/js-ipfsd-ctl instead of handling the daemon ourselves with execa calls.

@stale
Copy link

stale bot commented Mar 2, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for contributing to Aragon! 🦅

@stale stale bot added the abandoned label Mar 2, 2020
@stale stale bot closed this Mar 9, 2020
@macor161 macor161 reopened this Mar 9, 2020
@stale stale bot removed the abandoned label Mar 9, 2020
@0xGabi
Copy link
Contributor

0xGabi commented Mar 18, 2020

We will not pursue this new feature improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📟 cli A change related with cli tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IPFS init: checking whether .ipfs exists is brittle (e.g. when IPFS is installed via SNAP)
3 participants