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

feat: ipfs shutdown #1200

Merged
merged 11 commits into from
Feb 15, 2018
Merged

feat: ipfs shutdown #1200

merged 11 commits into from
Feb 15, 2018

Conversation

richardschneider
Copy link
Contributor

See issue #1192

@ghost ghost added the status/in-progress In progress label Jan 30, 2018
package.json Outdated
@@ -121,7 +121,7 @@
"libp2p-circuit": "~0.1.4",
"libp2p-floodsub": "~0.13.1",
"libp2p-kad-dht": "~0.6.0",
"libp2p-mdns": "~0.9.1",
"libp2p-mdns": "^0.9.2",
Copy link
Member

Choose a reason for hiding this comment

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

keep the use of ~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, that was npm install and poor review by me. Will fix.

@richardschneider
Copy link
Contributor Author

Will rebase tomorrow.

@daviddias
Copy link
Member

@richardschneider since this exists for go and for js ipfs, can you make tests on interface-ipfs-core and document the API call?

@richardschneider
Copy link
Contributor Author

Did documentation, from IRC

daviddias: accidentally did commit to master ipfs-inactive/interface-js-ipfs-core@9d91267
9:50 AM Sorry, it's no biggi just documentation update

README.md Outdated
@@ -325,6 +325,7 @@ A complete API definition is in the works. Meanwhile, you can learn how to you u
- [miscellaneous operations](https://github.com/ipfs/interface-ipfs-core/tree/master/SPEC/MISCELLANEOUS.md)
- [`ipfs.id([callback])`](https://github.com/ipfs/interface-ipfs-core/tree/master/SPEC/MISCELLANEOUS.md#id)
- [`ipfs.version([callback])`](https://github.com/ipfs/interface-ipfs-core/tree/master/SPEC/MISCELLANEOUS.md#version)
- [`ipfs.shutdown([callback])`](https://github.com/ipfs/interface-ipfs-core/tree/master/SPEC/MISCELLANEOUS.md#shutdown)
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking more about this. shutdown is a c&p of stop. Can we use just stop instead?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we could change it to stop. It's not a c&p, it's even simpler: this.shutdown = this.stop.

However, the /api/v0/shutdown end-point is already defined by go, so we must support it.

Also what do we do about the CLI; jsipfs shutdown or jsipfs stop or both?

Copy link
Member

Choose a reason for hiding this comment

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

@richardschneider let's do the this.shutdown = this.stop for now and keep the CLI as just shutdown.

Note, the HTTP API doesn't have to match entirely to the Core API which doesn't have to match the CLI API. It should enable the users to do the same operations, but we do have to have into account that they are used in different contexts. We will need to revisit all of the APIs later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@diasdavid I'm very confused. Is this PR now okay? What about ipfs-inactive/js-ipfs-http-client#685?

Copy link
Member

Choose a reason for hiding this comment

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

Almost.

The spec should be ipfs.stop as it is on https://github.com/ipfs/interface-ipfs-core/pull/215/files and we can have an alias for ipfs.shutdown in both js-ipfs and js-ipfs-api just for the sake of convenience.

Similar to how we do alias on https://github.com/ipfs/js-ipfs-api/#files

@richardschneider
Copy link
Contributor Author

@diasdavid This is RTM. Failures are in other tests.

@daviddias
Copy link
Member

@RichardLitt Circle CI disagrees

The error comes from the code that changed here. However, why are MFS tests running against js-ipfs, @hacdias ?

image

@RichardLitt
Copy link
Member

Ping @richardschneider

@richardschneider
Copy link
Contributor Author

@RichardLitt you can email on makaretu (at) gmail

@RichardLitt
Copy link
Member

I, uh, don't need to. @diasdavid pinged the wrong richard. :)

@daviddias daviddias changed the title Implement shutdown feat: ipfs shutdown Feb 15, 2018
@daviddias daviddias merged commit 95365fa into master Feb 15, 2018
@ghost ghost removed the status/in-progress In progress label Feb 15, 2018
@daviddias daviddias deleted the shutdown branch February 15, 2018 10:38
JonKrone pushed a commit that referenced this pull request Feb 16, 2018
* feat: route to shutdown daemon

* feat: cli and http-api shutdown

* chore: argggh fix the command count

* chore: merge conflicts

* chore: rebasing

* chore: fix rebase errors

* docs: add ipfs.shutdown

* fix: always report state error when stopping

* chore: documentation is 'stop' not 'shutdown'

* fix: generic stop

* fix: package.json
JonKrone pushed a commit to JonKrone/js-ipfs that referenced this pull request Feb 19, 2018
* feat: route to shutdown daemon

* feat: cli and http-api shutdown

* chore: argggh fix the command count

* chore: merge conflicts

* chore: rebasing

* chore: fix rebase errors

* docs: add ipfs.shutdown

* fix: always report state error when stopping

* chore: documentation is 'stop' not 'shutdown'

* fix: generic stop

* fix: package.json
dryajov pushed a commit that referenced this pull request Feb 28, 2018
* feat: route to shutdown daemon

* feat: cli and http-api shutdown

* chore: argggh fix the command count

* chore: merge conflicts

* chore: rebasing

* chore: fix rebase errors

* docs: add ipfs.shutdown

* fix: always report state error when stopping

* chore: documentation is 'stop' not 'shutdown'

* fix: generic stop

* fix: package.json
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants