coreapi: Basic object API implementation by magik6k · Pull Request #4492 · ipfs/kubo

magik6k · 2017-12-14T22:57:08Z

TODO:

Review
Docs
Tests

magik6k · 2017-12-14T22:58:58Z

core/coreapi/interface/interface.go

-// 	DataSize       int
-// 	CumulativeSize int
-// }
+	AddLink(ctx context.Context, base Path, name string, child Path, create bool) (Node, error) //TODO: make create optional


I'll make this optional after the approach to optional params in #4477 is reviewed/accepted

No need for arg names in an interface definition

magik6k · 2017-12-14T23:00:26Z

core/coreapi/object.go

+}
+
+func (api *ObjectAPI) Put(context.Context, coreiface.Node) error {
+	return errors.New("todo") // TODO: what should this method take? Should we just redir to dag-put?f


I'm not even sure if we want to keep this method, it seems to me that all of it's use cases are covered by Dag.Put

We should still implement it -- it can call DagAPI.Put() underneath

ghost · 2017-12-17T00:34:50Z

core/coreapi/interface/interface.go

+	Get(context.Context, Path) (Node, error)
+	Data(context.Context, Path) (io.Reader, error)
+	Links(context.Context, Path) ([]*Link, error)
+	Stat(context.Context, Path) (*ObjectStat, error)


I've been pondering in #4418 whether we should return a resolved Path in almost any place where we already have it resolved internally anyway. In some cases it saves a ton of additional resolve roundtrips, imagine we use the returned Path to pass it directly into another Core API function.

In addition, this is also useful in the case of functions that modify existing objects, e.g. SetData or AddLink. If I call it with /ipfs/QmFoo/a/b/c, I'd like it to return /ipfs/QmNewHash/a/b/c.

(Regardless of that, Put() should definitely return a Path.)

ghost · 2017-12-17T00:37:14Z

core/coreapi/object.go

+type ObjectAPI CoreAPI
+
+func (api *ObjectAPI) New(ctx context.Context) (coreiface.Node, error) {
+	node := new(dag.ProtoNode)


On the CLI/API, object new takes a template argument which can be unixfs-dir for a directory. We can do this here similarly to the KeyAPI.WithType() option in #4477.

ghost · 2017-12-17T00:41:18Z

core/coreapi/object.go

+}
+
+func (api *ObjectAPI) AddLink(ctx context.Context, base coreiface.Path, name string, child coreiface.Path, create bool) (coreiface.Node, error) {
+	rootNd, err := api.core().ResolveNode(ctx, base)


Let's call these variables base instead of root -- the root of /ipfs/QmFoo/a/b would be QmFoo, but the base of this operation is b

ghost

This looks super good :) Left a comment or two. I think that's all the object stuff needed for extracting the gateway, thanks!

magik6k · 2018-01-05T00:26:04Z

core/coreapi/interface/interface.go

+}
+
+// ObjectStat provides information about dag nodes
+type ObjectStat struct {


Should I wrap this into an interface for consistency?

I think it's fine - given that the object API is quasi-frozen (it's the old merkledag/dag-pb structure), it's very unlikely this struct will ever have any functions attached to it.

ghost · 2018-01-12T02:14:53Z

core/coreapi/interface/options/object.go

+
+func ObjectNewOptions(opts ...ObjectNewOption) (*ObjectNewSettings, error) {
+	options := &ObjectNewSettings{
+		Type: "empty",


The default for this should be "unixfs-dir" as in ipfs object new --help

The default for this should be "unixfs-dir" as in ipfs object new --help

Ah, nevermind, I understand :)

ghost · 2018-01-12T02:19:40Z

This LGTM 👍 - pretty cool!

Next let's extract coreiface to its own package, so we can get go-ipfs-api going.

License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>

magik6k added the topic/core-api Topic core-api label Dec 14, 2017

magik6k requested a review from a user December 14, 2017 22:57

ghost assigned magik6k Dec 14, 2017

ghost added the status/in-progress In progress label Dec 14, 2017

magik6k commented Dec 14, 2017

View reviewed changes

magik6k mentioned this pull request Dec 15, 2017

CoreAPI progress tracking issue #4498

Open

51 tasks

ghost reviewed Dec 17, 2017

View reviewed changes

ghost suggested changes Dec 17, 2017

View reviewed changes

magik6k force-pushed the feat/coreapi/object branch from 0c5d07c to 9713dff Compare December 22, 2017 16:25

ghost approved these changes Dec 23, 2017

View reviewed changes

magik6k force-pushed the feat/coreapi/object branch 2 times, most recently from 29e178a to 2ac9768 Compare January 1, 2018 16:47

magik6k force-pushed the feat/coreapi/object branch from 5b58fed to a65317d Compare January 5, 2018 00:17

magik6k commented Jan 5, 2018

View reviewed changes

ghost reviewed Jan 12, 2018

View reviewed changes

magik6k added 5 commits January 25, 2018 18:25

coreapi: Basic object API implementation

529f847

License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>

coreapi: Object api review

ae949e3

License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>

coreapi: object docs

64e2327

License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>

coreapi: implement object.Put

f6663bd

License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>

coreapi: object API tests

0377e49

License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>

magik6k force-pushed the feat/coreapi/object branch from a65317d to 0377e49 Compare January 25, 2018 17:27

whyrusleeping approved these changes Jan 30, 2018

View reviewed changes

whyrusleeping merged commit 4d8b3c9 into master Jan 30, 2018

ghost removed the status/in-progress In progress label Jan 30, 2018

whyrusleeping deleted the feat/coreapi/object branch January 30, 2018 21:14

Uh oh!

Conversation

magik6k commented Dec 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Dec 17, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jan 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

magik6k commented Dec 14, 2017 •

edited

Loading

ghost Dec 17, 2017 •

edited by ghost

Loading