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

Use go-ipfs-http-client for request handling #182

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

Conversation

magik6k
Copy link
Member

@magik6k magik6k commented May 10, 2019

This code is pretty much the same in both places, this should make the maintenance easier.

Fixes ipfs/go-ipfs-http-client#5, Depends on ipfs/go-ipfs-http-client#18

@ghost ghost assigned magik6k May 10, 2019
@ghost ghost added the status/in-progress In progress label May 10, 2019
@magik6k magik6k requested a review from Stebalien May 10, 2019 14:55
@Stebalien
Copy link
Member

cc @postables. Does this break anything?

@Stebalien
Copy link
Member

@magik6k could you test this against tools like ipget? If that still works, I'm fine going ahead with this.

@bonedaddy
Copy link
Contributor

Finally noticed the cc and got around to testing some of the tools I have that use go-ipfs-api. This change breaks some functions of tools I have written that have non-standard ways of using go-ipfs-api.

I don't really see this as an issue, certainly not something worth trying to prevent this PR from being merged simply because:

  • It's non standard code that isn't part of upstream (ipfs/go-ipfs-api) which should be expected to break at some point in time because its not officially part of the codebase
  • I'm probably the only one, or only one of a few people using these non-standard methods
  • The benefits of this PR vastly outweigh any perceived benefits from unofficial code

If desired I can follow this post up with examples of the non-standard functions I'm using. For clarification, all the errors that I'm getting are failed to connect to ipfs node at '<insert-ip-address>': operation not supported

tl;dr

LGTM but it may effect some people using this library in ways that aren't part of ipfs/go-ipfs-api however this is probably a non-issue

@Stebalien
Copy link
Member

@postables some examples would definitely be helpful.

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

Successfully merging this pull request may close these issues.

Extract request handling code
3 participants