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

feat: adapted to new ipfs-repo API #887

Merged
merged 4 commits into from
Jul 4, 2017
Merged

feat: adapted to new ipfs-repo API #887

merged 4 commits into from
Jul 4, 2017

Conversation

pgte
Copy link
Contributor

@pgte pgte commented Jun 30, 2017

ipfs-repo dependents are:

  • js-ipfs, this PR.
  • ipfs-block-service - PR
  • ipld-resolver - PR
  • ipfs-bitswap - PR

@Kubuxu Kubuxu added the status/in-progress In progress label Jun 30, 2017
@pgte pgte changed the title adapted to new ipfs-repo API WIP: adapted to new ipfs-repo API Jun 30, 2017
@pgte pgte requested a review from daviddias June 30, 2017 09:38
@pgte
Copy link
Contributor Author

pgte commented Jun 30, 2017

Depends on ipfs/js-ipfs-repo#140

@daviddias
Copy link
Member

@pgte please run npm run test:interop too

@pgte
Copy link
Contributor Author

pgte commented Jul 1, 2017

@diasdavid tested and it runs locally, but first you need to link these to the respective branches:

$ npm link ipfs-repo ipfs-block-service ipld-resolver ipfs-bitswap

@@ -15,23 +12,9 @@ module.exports = function config (self) {

if (!key) {
return self._repo.config.get(callback)
} else {
return self._repo.config.get(key, callback)
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the IPFS repo figure this out by itself now?

@@ -42,13 +25,7 @@ module.exports = function config (self) {
return callback(new Error('Invalid value type'))
}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this validation happening inside the repo itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should. ipfs/js-ipfs-repo#140 supports this now (via ipfs/js-ipfs-repo@8e8055d).
Fixed.

@daviddias
Copy link
Member

@pgte tests fail because of:

/Users/koruza/code/js-ipfs/src/core/components/start.js:51
      self._blockService.goOnline(self._bitswap)
                         ^

TypeError: self._blockService.goOnline is not a function
    at series (/Users/koruza/code/js-ipfs/src/core/components/start.j

Wasn't this change supposed to be contemplated in the awesome endeavor?

@daviddias daviddias changed the title WIP: adapted to new ipfs-repo API feat: adapted to new ipfs-repo API Jul 4, 2017
@daviddias daviddias merged commit 4e39d2c into master Jul 4, 2017
@daviddias daviddias deleted the chore/repo-new-api branch July 4, 2017 20:35
@daviddias daviddias removed the status/in-progress In progress label Jul 4, 2017
@pgte
Copy link
Contributor Author

pgte commented Jul 4, 2017

@diasdavid I hadn't included this change in the awesome endeavour, it was in a separate PR which wasn't listed here, I didn't think you would merge it as part of this.
Anyway, thanks for making this change!

dryajov pushed a commit that referenced this pull request Sep 1, 2017
* feat: adapted to new ipfs-repo API
MicrowaveDev pushed a commit to galtproject/js-ipfs that referenced this pull request May 22, 2020
BREAKING CHANGE. Previously swarm.peers would throw an uncaught error
if any peer in the reponse could have its peerId or multiaddr validated.

This PR catches errors that occur while validating the peer info. The
returned array will contain an entry for every peer in the ipfs response.
peer-info objects that couldn't be validated, now have an `error` property
and a `rawPeerInfo` property. This at least means the count of peers in
the response will be accurate, and there the info is available to the caller.

This means that callers now have to deal with peer-info objects that may
not have a `peer or `addr` property.

Adds `nock` tests to exercice the code under different error conditions.
Doing so uncovered a bug in our legacy go-ipfs <= 0.4.4 peer info parsing,
which is also fixed. The code was trying to decapusalate the peerId from
the multiaddr, but doing so trims the peerId rather than returning it.

fixes ipfs#885

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants