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.

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

core/commands: Make IpnsCmd and PublishCmd public #1397

Merged
merged 1 commit into from
Jun 20, 2015

Conversation

wking
Copy link
Contributor

@wking wking commented Jun 19, 2015

ipfs-shell accesses the Command objects directly to construct
requests for an external IPFS daemon API. This isn't a terribly
robust approach, because it doesn't handle version differences between
the version of go-ipfs used to build the daemon and the version used
to build the ipfs-shell-consuming application. But for cases where
you can get those APIs to match it works well. Making these two
commands public allows us to write ipfs-shell wrappers for them.
Until we figure out how to get ipfs-shell working without access to
core/commands, I think the best approach is to make future command
objects and their returned structures public, and to go back and
expose existing commands/structures on an as-needed basis.

In this case, I need the public PublishCmd for the Docker-registry
storage driver, and I made the IpnsCmd public at the same time to stay
consistent for both ipfs name ... sub-commands.

ipfs-shell [1] accesses the Command objects directly to construct
requests for an external IPFS daemon API.  This isn't a terribly
robust approach, because it doesn't handle version differences between
the version of go-ipfs used to build the daemon and the version used
to build the ipfs-shell-consuming application.  But for cases where
you can get those APIs to match it works well.  Making these two
commands public allows us to write ipfs-shell wrappers for them.
Until we figure out how to get ipfs-shell working without access to
core/commands, I think the best approach is to make future command
objects and their returned structures public, and to go back and
expose existing commands/structures on an as-needed basis.

In this case, I need the public PublishCmd for the Docker-registry
storage driver, and I made the IpnsCmd public at the same time to stay
consistent for both 'ipfs name ...' sub-commands.

[1]: https://github.com/whyrusleeping/ipfs-shell

License: MIT
Signed-off-by: W. Trevor King <wking@tremily.us>
@jbenet jbenet added the status/in-progress In progress label Jun 19, 2015
@wking
Copy link
Contributor Author

wking commented Jun 19, 2015

It's possible that we can extract this information from core/commands' Root object, in which case we can close this PR. I haven't looked through the commands tooling to see if this is possible yet. Current ipfs-shell usage looks like (error checking removed):

ropts, err := cc.Root.GetOptions([]string{"ls"})
req, err := cmds.NewRequest([]string{"ls", path}, nil, nil, nil, cc.LsCmd, ropts)
resp, err := s.client.Send(req)
read, err := resp.Reader()
dec := json.NewDecoder(read)
out := struct{ Objects []cc.Object }{}
err = dec.Decode(&out)

so we'd need a generic way to replace the access to cc.<command> (cc.LsCmd above) and cc.<returned-type> (struct{Objects []cc.Object} above).

@jbenet
Copy link
Member

jbenet commented Jun 19, 2015

May be able to get it from root. but I'm in favor of these commands becoming public anyway. They could be split out in own repos eventually too.

@jbenet
Copy link
Member

jbenet commented Jun 19, 2015

LGTM.

jbenet added a commit that referenced this pull request Jun 20, 2015
core/commands: Make IpnsCmd and PublishCmd public
@jbenet jbenet merged commit ed5374d into master Jun 20, 2015
@jbenet jbenet removed the status/in-progress In progress label Jun 20, 2015
@jbenet jbenet deleted the tk/public-name-commands branch June 20, 2015 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants