Support PinPath, UnpinPath (resolve before pinning) #634

kishansagathiya · 2019-01-09T13:58:20Z

This PR adds API support for pinning using path

POST /pins/<ipfs or ipns path> and DELETE /pins/<ipfs or ipns path>
will resolve the path into a cid and perform perform pinning or
unpinning

To do

add the same support in command
tests

Fixes #450

This commit adds API support for pinning using path `POST /pins/<ipfs or ipns path>` and `DELETE /pins/<ipfs or ipns path>` will resolve the path into a cid and perform perform pinning or unpinning Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

License: MIT Signed-off-by: cd10012 <ced361@nyu.edu>

This reverts commit 28a3a3f. License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

coveralls · 2019-01-09T14:11:36Z

Coverage decreased (-0.05%) to 65.795% when pulling aaada42 on issue_450 into cc8dd7e on master.

api/rest/restapi.go

kishansagathiya · 2019-01-10T09:32:13Z

Are we following any pattern on when to return err and when to log and return? The code that I am dealing with has a bit inconsistent error logging.

hsanjuan · 2019-01-10T10:52:04Z

Are we following any pattern on when to return err and when to log and return? The code that I am dealing with has a bit inconsistent error logging.

I'd say log errors when something should have worked but did not. Do not log errors because the user gave a bad input, just return it to them.

And usually, log in the place where it makes more sense to be logged and it's useful (closer to the source of the error) and try to log once and not everytime the error is bubbled up through the stack (although it really depends on the context).

This all amounts to: no pattern followed

kishansagathiya · 2019-01-10T12:30:12Z

Changing client methods Pin and Unpin to take path instead of cid.
Would you rather have separate PinPath and UnpinPath there as well?

hsanjuan · 2019-01-11T10:57:55Z

Would you rather have separate PinPath and UnpinPath there as well?

Yes.

- Separate handlers, methods and rpc apis for PinPath and UnpinPath from Pin and Unpin - Support ipld paths as well Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

kishansagathiya · 2019-01-14T11:26:05Z

PinPath and UnpinPath would return the resolved cid, because later parts in the command requires it.

hsanjuan

Some more comments

ipfsconn/ipfshttp/ipfshttp.go

api/rest/restapi.go

- IPFS Connector should act as a pure IPFS client, any extra logic should go to cluster.go - Use cid.Undef, instead of cid.Cid{} Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

api/rest/restapi.go

Tests Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

error strings should not be capitalized Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

api/rest/client/methods_test.go

Extra logic for path checking should go into resolve so that it can be properly reused Added sharness tests License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

api/rest/client/methods.go

api/rest/client/methods_test.go

api/rest/restapi.go

rpc_api.go

sharness/t0030-ctl-pin.sh

- RPC must always use serializable structs - In command, just use pin with path as cid is also a valid path - Addressing many other small review comments as in #634 (review) License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

PinPath(in both rest and rpc) should return a serializable struct in the form `{"\":"Q...cid..string..."}` (as used in "github.com/ipfs/go-cid" to marshal and unmarshal) License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

old

kishansagathiya · 2019-02-13T15:34:44Z

🎉 🎊

api/rest/client/methods.go

lanzafame · 2019-02-18T00:45:28Z

api/rest/restapi.go

+	var pin types.PinSerial
+	if pinpath := api.parsePinPathOrError(w, r); pinpath.Path != "" {
+		logger.Debugf("rest api pinPathHandler: %s", pinpath.Path)
+		err := api.rpcClient.Call(


Change to CallContext and hand r.Context() as the context parameter.

lanzafame · 2019-02-18T00:45:50Z

api/rest/restapi.go

+	var pin types.PinSerial
+	if pinpath := api.parsePinPathOrError(w, r); pinpath.Path != "" {
+		logger.Debugf("rest api unpinPathHandler: %s", pinpath.Path)
+		err := api.rpcClient.Call(


Change to CallContext and hand r.Context() as the context parameter.

cluster.go

cmd/ipfs-cluster-ctl/main.go

ipfsconn/ipfshttp/ipfshttp.go

Added tracing for new methods

hsanjuan · 2019-02-19T13:33:20Z

@lanzafame is this good for you?

@kishansagathiya can you manually verify the different ways of calling ipfs-cluster-ctl pin add/rm work, produce the right output (including --enc json) and confirm that we're good to merge?

kishansagathiya · 2019-02-19T16:48:44Z

@hsanjuan Commands run as expected. Let's wait for the checks to pass, this should be good to go then.

hsanjuan · 2019-02-19T21:45:34Z

Tests fail because master is broken at the moment waiting on #655 , but other than that it's good.

lanzafame

👍

Squashed commit of the following: commit 38cf569 Merge: d125f69 aaada42 Author: Hector Sanjuan <hsanjuan@users.noreply.github.com> Date: Wed Feb 20 11:02:00 2019 +0000 Merge pull request #634 from ipfs/issue_450 Support PinPath, UnpinPath (resolve before pinning) commit aaada42 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Feb 19 22:16:25 2019 +0530 formatResponse accepts api.Pin and not api.PinSerial commit b5da4be Merge: ba59036 cc8dd7e Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Feb 19 21:36:46 2019 +0530 Merge branch 'master' into issue_450 commit ba59036 Merge: f002914 d59880c Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Mon Feb 18 08:41:11 2019 +0530 Merge branch 'issue_450' of github.com:ipfs/ipfs-cluster into issue_450 commit f002914 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Mon Feb 18 08:31:39 2019 +0530 PinPath: more improvements Added tracing for new methods commit d59880c Merge: 0ca4c7c b4f0eb3 Author: Hector Sanjuan <hsanjuan@users.noreply.github.com> Date: Wed Feb 13 15:22:49 2019 +0000 Merge branch 'master' into issue_450 commit 0ca4c7c Merge: d35017a ecef9ea Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Feb 12 13:10:13 2019 +0530 Merge branch 'master' into issue_450 commit d35017a Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Feb 12 13:07:25 2019 +0530 PinPath: more improvements - Worth having `PinOptions` as a separate field in the struct and constructing the query in the test with ToQuery() - sharness: "intialization" line can be placed outside the tests at the top commit 68e3b90 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Sun Feb 10 21:43:50 2019 +0530 Using if-continue pattern instead of if-else License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 3c29799 Merge: 956790b 4324889 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Thu Feb 7 10:25:52 2019 +0530 Merge branch 'master' into issue_450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 956790b Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Feb 6 21:11:20 2019 +0530 Removing resolved path License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 7191cc4 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Feb 6 16:45:07 2019 +0530 Fix go vet License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit f8b3d5b Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Feb 6 16:07:03 2019 +0530 Fixed linting error License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 23c57eb Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Feb 6 09:20:41 2019 +0530 Fixed tests License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 0caedd9 Merge: 17e555e 5a7ee1d Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Feb 6 00:07:10 2019 +0530 Merge branch 'master' into issue_450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 17e555e Author: Hector Sanjuan <code@hector.link> Date: Tue Feb 5 16:58:50 2019 +0000 PinPath: address some feedback + improvements * Changed client's Pin() API and PinPath to be consistent * Added helper methods to turn PinPath to query and back * Make code and tests build * Use TestCidResolved everywhere * Fix cluster.PinPath arguments * Fix formatting of responses with --no-status * Make tests readable and call Fatal when needed * Use a pathTestCases variable commit f0e7369 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Feb 5 18:34:26 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) Addressed review comments as in #634 (review) License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit a8b4f18 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Jan 29 22:41:27 2019 +0530 Fixing tests License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit e39b95c Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Jan 29 14:52:53 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) - PinPath and UnpinPath should return api.Pin - PinPath should accept pin options - Removing duplicate logic for Resolve from cluster - And many other review comments #634 (review) License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit d146075 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Jan 23 17:08:41 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) PinPath(in both rest and rpc) should return a serializable struct in the form `{"\":"Q...cid..string..."}` (as used in "github.com/ipfs/go-cid" to marshal and unmarshal) License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 1f48695 Merge: 7acfd28 a244af9 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Jan 23 07:18:56 2019 +0530 Merge branch 'master' into issue_450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 7acfd28 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Jan 22 18:14:32 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) - RPC must always use serializable structs - In command, just use pin with path as cid is also a valid path - Addressing many other small review comments as in #634 (review) License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 3690504 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Jan 16 09:36:42 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) Extra logic for path checking should go into resolve so that it can be properly reused Added sharness tests License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 9116bda Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Jan 16 08:08:07 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) error strings should not be capitalized Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit ca7e618 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Jan 15 23:40:25 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) Tests Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 522fbcd Merge: f1a56ab f7bc468 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Jan 15 10:40:54 2019 +0530 Merge branch 'master' into issue_450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit f1a56ab Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Mon Jan 14 20:58:17 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) - IPFS Connector should act as a pure IPFS client, any extra logic should go to cluster.go - Use cid.Undef, instead of cid.Cid{} Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit c83b910 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Thu Jan 10 08:57:17 2019 +0530 Support PinPath, UnpinPath(resolve before pinning) - Separate handlers, methods and rpc apis for PinPath and UnpinPath from Pin and Unpin - Support ipld paths as well Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 719dff8 Merge: 91ceb47 21170c4 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Jan 9 19:38:35 2019 +0530 Merge branch 'issue_450_old' into HEAD License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 91ceb47 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Wed Jan 9 19:36:41 2019 +0530 Revert "WIP: Figure out why test does not impleme" This reverts commit 28a3a3f. License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> commit 28a3a3f Author: cd10012 <ced361@nyu.edu> Date: Tue Jul 24 23:23:10 2018 -0400 WIP: Figure out why test does not implement IPFSConnector interface... License: MIT Signed-off-by: cd10012 <ced361@nyu.edu> commit 21170c4 Author: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Date: Tue Jan 8 10:37:59 2019 +0530 Support PinPath, UnpinPath (resolve before pinning) This commit adds API support for pinning using path `POST /pins/<ipfs or ipns path>` and `DELETE /pins/<ipfs or ipns path>` will resolve the path into a cid and perform perform pinning or unpinning Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com> Co-authored-by: Hector Sanjuan <hector@protocol.ai> License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>

hsanjuan · 2019-02-20T11:18:20Z

I merged too quickly and wanted to squash this, so did it manually on master :/

ghost assigned kishansagathiya Jan 9, 2019

ghost added the status/in-progress In progress label Jan 9, 2019

corydickson and others added 3 commits January 9, 2019 19:36

WIP: Figure out why test does not implement IPFSConnector interface...

28a3a3f

License: MIT Signed-off-by: cd10012 <ced361@nyu.edu>

Revert "WIP: Figure out why test does not impleme"

91ceb47

This reverts commit 28a3a3f. License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

Merge branch 'issue_450_old' into HEAD

719dff8

License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

kishansagathiya force-pushed the issue_450 branch from c6b9f06 to 719dff8 Compare January 9, 2019 14:10

kishansagathiya mentioned this pull request Jan 9, 2019

WIP: Learn how to pin using cid and what to do with file path #493

Closed

hsanjuan requested changes Jan 9, 2019

View reviewed changes

api/rest/restapi.go Outdated Show resolved Hide resolved

api/rest/restapi.go Outdated Show resolved Hide resolved

api/rest/restapi.go Outdated Show resolved Hide resolved

Support PinPath, UnpinPath(resolve before pinning)

c83b910

- Separate handlers, methods and rpc apis for PinPath and UnpinPath from Pin and Unpin - Support ipld paths as well Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

kishansagathiya force-pushed the issue_450 branch from ee45e25 to c83b910 Compare January 14, 2019 05:17

hsanjuan reviewed Jan 14, 2019

View reviewed changes

kishansagathiya added 2 commits January 15, 2019 10:39

Support PinPath, UnpinPath(resolve before pinning)

f1a56ab

- IPFS Connector should act as a pure IPFS client, any extra logic should go to cluster.go - Use cid.Undef, instead of cid.Cid{} Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

Merge branch 'master' into issue_450

522fbcd

License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

kishansagathiya commented Jan 15, 2019

View reviewed changes

api/rest/restapi.go Show resolved Hide resolved

kishansagathiya added 2 commits January 15, 2019 23:40

Support PinPath, UnpinPath(resolve before pinning)

ca7e618

Tests Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

Support PinPath, UnpinPath(resolve before pinning)

9116bda

error strings should not be capitalized Fixes #450 License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

kishansagathiya commented Jan 16, 2019

View reviewed changes

api/rest/client/methods_test.go Outdated Show resolved Hide resolved

Support PinPath, UnpinPath(resolve before pinning)

3690504

Extra logic for path checking should go into resolve so that it can be properly reused Added sharness tests License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

kishansagathiya force-pushed the issue_450 branch from cca8534 to 3690504 Compare January 16, 2019 05:37

hsanjuan requested changes Jan 17, 2019

View reviewed changes

kishansagathiya added 3 commits January 23, 2019 07:18

Merge branch 'master' into issue_450

1f48695

License: MIT Signed-off-by: Kishan Mohanbhai Sagathiya <kishansagathiya@gmail.com>

ipfs-cluster deleted a comment from GitCop Feb 13, 2019

Merge branch 'master' into issue_450

d59880c

ipfs-cluster deleted a comment from GitCop Feb 13, 2019

hsanjuan approved these changes Feb 13, 2019

View reviewed changes

hsanjuan requested a review from lanzafame February 13, 2019 15:29

lanzafame suggested changes Feb 18, 2019

View reviewed changes

kishansagathiya added 2 commits February 18, 2019 08:32

PinPath: more improvements

f002914

Added tracing for new methods

Merge branch 'issue_450' of github.com:ipfs/ipfs-cluster into issue_450

ba59036

ipfs-cluster deleted a comment from GitCop Feb 19, 2019

kishansagathiya added 2 commits February 19, 2019 21:36

Merge branch 'master' into issue_450

b5da4be

formatResponse accepts api.Pin and not api.PinSerial

aaada42

lanzafame approved these changes Feb 20, 2019

View reviewed changes

hsanjuan merged commit 38cf569 into master Feb 20, 2019

ghost removed the status/in-progress In progress label Feb 20, 2019

hsanjuan deleted the issue_450 branch February 20, 2019 11:02

hsanjuan restored the issue_450 branch February 20, 2019 11:03

hsanjuan deleted the issue_450 branch February 20, 2019 11:17

hsanjuan mentioned this pull request Feb 20, 2019

Release 0.10.0 #678

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support PinPath, UnpinPath (resolve before pinning) #634

Support PinPath, UnpinPath (resolve before pinning) #634

kishansagathiya commented Jan 9, 2019

coveralls commented Jan 9, 2019 •

edited

kishansagathiya commented Jan 10, 2019 •

edited

hsanjuan commented Jan 10, 2019

kishansagathiya commented Jan 10, 2019

hsanjuan commented Jan 11, 2019

kishansagathiya commented Jan 14, 2019

hsanjuan left a comment

kishansagathiya commented Feb 13, 2019

lanzafame Feb 18, 2019

lanzafame Feb 18, 2019

hsanjuan commented Feb 19, 2019

kishansagathiya commented Feb 19, 2019

hsanjuan commented Feb 19, 2019 •

edited

lanzafame left a comment

hsanjuan commented Feb 20, 2019

Support PinPath, UnpinPath (resolve before pinning) #634

Support PinPath, UnpinPath (resolve before pinning) #634

Conversation

kishansagathiya commented Jan 9, 2019

coveralls commented Jan 9, 2019 • edited

kishansagathiya commented Jan 10, 2019 • edited

hsanjuan commented Jan 10, 2019

kishansagathiya commented Jan 10, 2019

hsanjuan commented Jan 11, 2019

kishansagathiya commented Jan 14, 2019

hsanjuan left a comment

Choose a reason for hiding this comment

kishansagathiya commented Feb 13, 2019

lanzafame Feb 18, 2019

Choose a reason for hiding this comment

lanzafame Feb 18, 2019

Choose a reason for hiding this comment

hsanjuan commented Feb 19, 2019

kishansagathiya commented Feb 19, 2019

hsanjuan commented Feb 19, 2019 • edited

lanzafame left a comment

Choose a reason for hiding this comment

hsanjuan commented Feb 20, 2019

coveralls commented Jan 9, 2019 •

edited

kishansagathiya commented Jan 10, 2019 •

edited

hsanjuan commented Feb 19, 2019 •

edited