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.

Sign up for GitHub

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

Jump to bottom

docs: improved --help for pin remote commands #7823

Merged
merged 3 commits into from
Jan 28, 2021
Merged

docs: improved --help for pin remote commands #7823

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b82dd8
docs: CLI docs for pin remote commands
lidel Dec 11, 2020
1cb90d7
docs: pin remote readability
lidel Dec 14, 2020
0b6912e
refactor: cleanup arg params
lidel Dec 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 96 additions & 23 deletions core/commands/pin/remotepin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ const pinNameOptionName = "name"
const pinCIDsOptionName = "cid"
const pinStatusOptionName = "status"
const pinServiceNameOptionName = "service"
const pinServiceURLOptionName = "url"
const pinServiceKeyOptionName = "key"
const pinServiceNameArgName = pinServiceNameOptionName
const pinServiceEndpointArgName = "endpoint"
const pinServiceKeyArgName = "key"
const pinServiceStatOptionName = "stat"
const pinBackgroundOptionName = "background"
const pinForceOptionName = "force"
Expand Down Expand Up @@ -89,20 +90,41 @@ func printRemotePinDetails(w io.Writer, out *RemotePinOutput) {

// remote pin commands

var pinServiceNameOption = cmds.StringOption(pinServiceNameOptionName, "Name of the remote pinning service to use.")
var pinServiceNameOption = cmds.StringOption(pinServiceNameOptionName, "Name of the remote pinning service to use (mandatory).")

var addRemotePinCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "Pin object to remote pinning service.",
ShortDescription: "Stores an IPFS object from a given path to a remote pinning service.",
ShortDescription: "Asks remote pinning service to pin an IPFS object from a given path.",
LongDescription: `
Asks remote pinning service to pin an IPFS object from a given path or a CID.

To pin CID 'bafkqaaa' to service named 'mysrv' under a pin named 'mypin':

$ ipfs pin remote add --service=mysrv --name=mypin bafkqaaa

The above command will block until remote service returns 'pinned' status,
which may take time depending on the size and available providers of the pinned
data.

If you prefer to not wait for pinning confirmation and return immediately
after remote service confirms 'queued' status, add the '--background' flag:

$ ipfs pin remote add --service=mysrv --name=mypin --background bafkqaaa

Status of background pin requests can be inspected with the 'ls' command:

$ ipfs pin remote ls --service=mysrv --cid=bafkqaaa --status=queued,pinning,pinned,failed
Copy link
Contributor

Choose a reason for hiding this comment

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

We support --status=queued --status=pinning ... via the StringsOption option.

We currently have some code that allow each --status=ABC that has commas in it to be flattened together, IMO this behavior is a little confusing and if we keep both then we should definitely document it.

@lidel why are we supporting the comma separated version is it for user ergonomics, compatibility with clients that refuse to send repeated HTTP query parameters or both?

If it's just for user ergonomics we may want to not tie ourselves to processing this server side and add this into the CLI processing for go-ipfs-cmds (e.g. https://github.com/ipfs/go-ipfs-cmds/blob/edc78ef36c1dadd4d1a87737e9a1727e59d36c45/cli/parse.go#L101).

Copy link
Member Author

@lidel lidel Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@aschmahmann
It is just for user ergonomics. I was playing with CLI and the alternative is pretty tedious.
I am considering adding --status=any (https://github.com/ipfs/go-pinning-service-http-client/issues/7) to improve it even further.

My concern with adding it as a global feature of go-ipfs-cmds is that it will block use of , in values, it becomes a value separator. Here, its fine, as , won't be part of status or cid, but we don't know if its equally safe for other places: what if I want to pass multiple unix file paths, and one of them has , in file or dir name?

I suggest we keep it local to pin remote commands to unblock 0.8.0.
We can refactor and figure out how to handle it globally in future releases.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about if we allowed passing an option into StringsOption with a delimiter. If the delimiter is defined (e.g. a comma, colon, etc) the it would be used, but otherwise you'd have to pass multiple options instead of using a delimiter

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that would be 👌 – removes risk I described.

Copy link
Contributor

@aschmahmann aschmahmann Dec 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Cool, I'll poke around go-ipfs-cmds and see how annoying this is to implement. Thinking it should be a little painful but not too bad.

Note: we'll need docs describing this behavior and that the delimiter exists and only applies to the CLI since the HTTP API docs are generated from these comments too 😬

Copy link
Contributor

Choose a reason for hiding this comment

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

This has been implemented in go-ipfs-cmds and the dep bubbled into go-ipfs, this PR can be rebased on master to take advantage of it. #7863


`,
},

Arguments: []cmds.Argument{
cmds.StringArg("ipfs-path", true, false, "Path to object(s) to be pinned."),
},
Options: []cmds.Option{
cmds.StringOption(pinNameOptionName, "An optional name for the pin."),
pinServiceNameOption,
cmds.StringOption(pinNameOptionName, "An optional name for the pin."),
cmds.BoolOption(pinBackgroundOptionName, "Add to the queue on the remote service and return immediately (does not wait for pinned status).").WithDefault(false),
},
Type: RemotePinOutput{},
Expand Down Expand Up @@ -218,15 +240,18 @@ Returns a list of objects that are pinned to a remote pinning service.
`,
LongDescription: `
Returns a list of objects that are pinned to a remote pinning service.

NOTE: By default, it will only show matching objects in 'pinned' state.
Pass '--status=queued,pinning,pinned,failed' to list pins in all states.
`,
},

Arguments: []cmds.Argument{},
Options: []cmds.Option{
cmds.StringOption(pinNameOptionName, "Return pins objects with names that contain provided value (case-sensitive, exact match)."),
cmds.DelimitedStringsOption(",", pinCIDsOptionName, "Return only pin objects for the specified CID(s); optional, comma separated."),
cmds.DelimitedStringsOption(",", pinStatusOptionName, "Return only pin objects with the specified statuses (queued,pinning,pinned,failed)").WithDefault([]string{"pinned"}),
pinServiceNameOption,
cmds.StringOption(pinNameOptionName, "Return pins with names that contain the value provided (case-sensitive, exact match)."),
cmds.DelimitedStringsOption(",", pinCIDsOptionName, "Return pins for the specified CIDs (comma-separated)."),
cmds.DelimitedStringsOption(",", pinStatusOptionName, "Return pins with the specified statuses (queued,pinning,pinned,failed).").WithDefault([]string{"pinned"}),
},
Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
ctx, cancel := context.WithCancel(req.Context)
Expand Down Expand Up @@ -300,20 +325,41 @@ func lsRemote(ctx context.Context, req *cmds.Request, c *pinclient.Client) (chan

var rmRemotePinCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "Remove pinned objects from remote pinning service.",
ShortDescription: `
Removes the pin from the given object allowing it to be garbage
collected if needed.
Tagline: "Remove pins from remote pinning service.",
ShortDescription: "Removes the remote pin, allowing it to be garbage-collected if needed.",
LongDescription: `
Removes remote pins, allowing them to be garbage-collected if needed.

This command accepts the same search query parameters as 'ls', and it is good
practice to execute 'ls' before 'rm' to confirm the list of pins to be removed.

To remove a single pin for a specific CID:

$ ipfs pin remote ls --service=mysrv --cid=bafkqaaa
$ ipfs pin remote rm --service=mysrv --cid=bafkqaaa

When more than one pin matches the query on the remote service, an error is
returned. To confirm the removal of multiple pins, pass '--force':

$ ipfs pin remote ls --service=mysrv --name=popular-name
$ ipfs pin remote rm --service=mysrv --name=popular-name --force

NOTE: When no '--status' is passed, implicit '--status=pinned' is used.
To list and then remove all pending pin requests, pass an explicit status list:

$ ipfs pin remote ls --service=mysrv --status=queued,pinning,failed
$ ipfs pin remote rm --service=mysrv --status=queued,pinning,failed --force

`,
},

Arguments: []cmds.Argument{},
Options: []cmds.Option{
pinServiceNameOption,
cmds.StringOption(pinNameOptionName, "Remove pin objects with names that contain provided value (case-sensitive, exact match)."),
cmds.DelimitedStringsOption(",", pinCIDsOptionName, "Remove only pin objects for the specified CID(s)."),
cmds.DelimitedStringsOption(",", pinStatusOptionName, "Remove only pin objects with the specified statuses (queued,pinning,pinned,failed).").WithDefault([]string{"pinned"}),
cmds.BoolOption(pinForceOptionName, "Remove multiple pins without confirmation.").WithDefault(false),
cmds.StringOption(pinNameOptionName, "Remove pins with names that contain provided value (case-sensitive, exact match)."),
cmds.DelimitedStringsOption(",", pinCIDsOptionName, "Remove pins for the specified CIDs."),
cmds.DelimitedStringsOption(",", pinStatusOptionName, "Remove pins with the specified statuses (queued,pinning,pinned,failed).").WithDefault([]string{"pinned"}),
cmds.BoolOption(pinForceOptionName, "Allow removal of multiple pins matching the query without additional confirmation.").WithDefault(false),
},
Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
ctx, cancel := context.WithCancel(req.Context)
Expand Down Expand Up @@ -358,12 +404,27 @@ collected if needed.
var addRemotePinServiceCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "Add remote pinning service.",
ShortDescription: "Add a credentials for access to a remote pinning service.",
ShortDescription: "Add credentials for access to a remote pinning service.",
LongDescription: `
Add credentials for access to a remote pinning service and store them in the
config under Pinning.RemoteServices map.

TIP:

To add services and test them by fetching pin count stats:

$ ipfs pin remote service add goodsrv https://pin-api.example.com secret-key
$ ipfs pin remote service add badsrv https://bad-api.example.com invalid-key
$ ipfs pin remote service ls --stat
goodsrv https://pin-api.example.com 0/0/0/0
badsrv https://bad-api.example.com invalid

`,
},
Arguments: []cmds.Argument{
cmds.StringArg(pinServiceNameOptionName, true, false, "Service name."),
cmds.StringArg(pinServiceURLOptionName, true, false, "Service URL."),
cmds.StringArg(pinServiceKeyOptionName, true, false, "Service key."),
cmds.StringArg(pinServiceNameArgName, true, false, "Service name."),
cmds.StringArg(pinServiceEndpointArgName, true, false, "Service endpoint."),
cmds.StringArg(pinServiceKeyArgName, true, false, "Service key."),
},
Type: nil,
Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
Expand All @@ -378,7 +439,7 @@ var addRemotePinServiceCmd = &cmds.Command{
defer repo.Close()

if len(req.Arguments) < 3 {
return fmt.Errorf("expecting three arguments: service name, url and key")
return fmt.Errorf("expecting three arguments: service name, endpoint and key")
}

name := req.Arguments[0]
Expand All @@ -387,7 +448,7 @@ var addRemotePinServiceCmd = &cmds.Command{

u, err := neturl.ParseRequestURI(url)
if err != nil || !strings.HasPrefix(u.Scheme, "http") {
return fmt.Errorf("service url must be a valid HTTP URL")
return fmt.Errorf("service endpoint must be a valid HTTP URL")
}

cfg, err := repo.Config()
Expand Down Expand Up @@ -419,7 +480,7 @@ var rmRemotePinServiceCmd = &cmds.Command{
ShortDescription: "Remove credentials for access to a remote pinning service.",
},
Arguments: []cmds.Argument{
cmds.StringArg("remote-pin-service", true, false, "Name of remote pinning service to remove."),
cmds.StringArg(pinServiceNameOptionName, true, false, "Name of remote pinning service to remove."),
lidel marked this conversation as resolved.
Show resolved Hide resolved
},
Options: []cmds.Option{},
Type: nil,
Expand Down Expand Up @@ -454,6 +515,18 @@ var lsRemotePinServiceCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "List remote pinning services.",
ShortDescription: "List remote pinning services.",
LongDescription: `
List remote pinning services.

By default, only a name and an endpoint are listed; however, one can pass
'--stat' to test each endpoint by fetching pin counts for each state:

$ ipfs pin remote service ls --stat
goodsrv https://pin-api.example.com 0/0/0/0
badsrv https://bad-api.example.com invalid

TIP: pass '--enc=json' for more useful JSON output.
`,
},
Arguments: []cmds.Argument{},
Options: []cmds.Option{
Expand Down