Internal organization for this repository (internal, core, shell, http, cmd, test) #1158

wking · 2015-04-28T17:57:04Z

On Mon, Apr 27, 2015 at 11:17:18PM -0700, Jeromy Johnson wrote:

In my perfect awesome world where I wrote all the code and spent
lots of time making the interfaces perfect, We would have the
importer, with all its knobs and switches on the low end (probably),
and coreunix.Add being all nice and user friendly on top, and then
the commands.Add would just call into coreunix.Add.

I guess the further refinement of that idea is that all commands
are just UX wrappers over the core* methods, and the core*
methods handle a bunch of logic in order to make calls to the low
level importer style, raw methods.
maybe we should rename core -> mantle, and call the lowest level type API commands the core...

So after a fairly brief chat on IRC with @jbenet, my understanding was
that we wanted something like:

internal/… Really low-level stuff where we reserve the right to
break interfaces as often as we like.
core/… The official, low-level API that gives consumers all the
knobs they need, which we try hard to keep stable.
shell/… The official, high-level API that gives consumers easy
access to common operations (e.g. create a file node from a reader
without wrapping with metadata). We work really hard to keep
this stable.

Then on top of the core/… and shell/… Go APIs, we'd build:

http/… The REST API
cmd/… The command-line API
test/… Integration tests, etc.

To avoid cyclic imports (see part of my commit message in ccb3f39
(util/testutil/addcat/addcat: Factor out add/cat/verify helper,
2015-04-26), I'd jump through whatever hoops had to be jumped through
to ensure imports never cut further down in that stack. For example,
you could import all of core and shell from cmd/… or test/…, but you
couldn't import any of shell/… from core/….

Spun off from ipfs/team-mgmt#6.

The text was updated successfully, but these errors were encountered:

jbenet · 2015-04-28T23:53:27Z

all SGTM.

except one comment:

we currently have some internal/ packages, but i'm strongly against them and want to get rid of them. they can cause really, really bad situations. (fun story: https://github.com/jbenet/go-fd-mutex -- this isn't quite internal/, but there are similar cases). these can just be packages and users can vendor them to avoid breaking on api changes. (i.e. we can be clear about what packages we promise not to change and which we do. internal/ is a very very annoying way to signal that to the user.

wking · 2015-04-29T00:20:37Z

On Tue, Apr 28, 2015 at 04:53:28PM -0700, Juan Batiz-Benet wrote:

we currently have some internal/ packages, but i'm strongly
against them and want to get rid of them.

I expect we'll have stuff that's so low-level that we can't imagine
folks using it directly that we don't want to commit to stabilizing.
It takes some work to deprecate nicely (see #1159), and if that
applies to all of our package-public interfaces I think it would be
too big a drag. But maybe we don't want anything that low-level ;).
Anyhow, I'm fine moving stuff into “we commit to trying to keep this
stable, and will work hard to help you migrate smoothly when we do
break backwards compatibility” locations as we see fit, and just
letting the internal stuff about which we don't make those guarantees
wither away.

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). [1]: ipfs#1158 [2]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/fsnode instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/fsnode version has a better API and we want to push people in that direction. The shell/fsnode version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2]. [1]: ipfs#1158 [2]: ipfs#1159

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). [1]: ipfs#1158 [2]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/fsnode instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/fsnode version has a better API and we want to push people in that direction. The shell/fsnode version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2]. [1]: ipfs#1158 [2]: ipfs#1159

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). I'm not clear on where the 'unix' part of the name comes from (these seem like pretty generic filesystem nodes to me, or if anything, they're POSIX filesystem nodes), but: On Wed, Apr 29, 2015 at 01:30:04PM -0700, Juan Batiz-Benet wrote [3]: > > package fsnode > > i think this package should be called unixfs as that's the > abstraction that this is calling to. So that's what we're going with. [1]: ipfs#1158 [2]: ipfs#1136 (comment) [3]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/unixfs instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/unixfs version has a better API and we want to push people in that direction. The shell/unixfs version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2]. [1]: ipfs#1158 [2]: ipfs#1159

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). I'm not clear on where the 'unix' part of the name comes from (these seem like pretty generic filesystem nodes to me, or if anything, they're POSIX filesystem nodes), but: On Wed, Apr 29, 2015 at 01:30:04PM -0700, Juan Batiz-Benet wrote [3]: > > package fsnode > > i think this package should be called unixfs as that's the > abstraction that this is calling to. So that's what we're going with. [1]: ipfs#1158 [2]: ipfs#1136 (comment) [3]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/unixfs instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/unixfs version has a better API and we want to push people in that direction. The shell/unixfs version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2], with the API-release-notes directory proposed here [3] and accepted here [4]. [1]: ipfs#1158 [2]: ipfs#1159 [3]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29379579 [4]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29498385

Discussion in [1]. [1]: ipfs#1158

Based on [1]. [1]: ipfs#1158

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). I'm not clear on where the 'unix' part of the name comes from (these seem like pretty generic filesystem nodes to me, or if anything, they're POSIX filesystem nodes), but: On Wed, Apr 29, 2015 at 01:30:04PM -0700, Juan Batiz-Benet wrote [3]: > > package fsnode > > i think this package should be called unixfs as that's the > abstraction that this is calling to. So that's what we're going with. [1]: ipfs#1158 [2]: ipfs#1136 (comment) [3]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/unixfs instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/unixfs version has a better API and we want to push people in that direction. The shell/unixfs version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2], with the API-release-notes directory proposed here [3] and accepted here [4]. [1]: ipfs#1158 [2]: ipfs#1159 [3]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29379579 [4]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29498385

wking · 2015-05-02T19:38:22Z

I recommend closing this issue now that 6fd877d (doc: Add some docs explaining the sub-package layout, 2015-05-02, #1185) has landed. Unless there are other places we want to document this layout?

jbenet · 2015-05-02T20:10:44Z

this is good enough for now

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). I'm not clear on where the 'unix' part of the name comes from (these seem like pretty generic filesystem nodes to me, or if anything, they're POSIX filesystem nodes), but: On Wed, Apr 29, 2015 at 01:30:04PM -0700, Juan Batiz-Benet wrote [3]: > > package fsnode > > i think this package should be called unixfs as that's the > abstraction that this is calling to. So that's what we're going with. [1]: ipfs#1158 [2]: ipfs#1136 (comment) [3]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/unixfs instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/unixfs version has a better API and we want to push people in that direction. The shell/unixfs version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2], with the API-release-notes directory proposed here [3] and accepted here [4]. [1]: ipfs#1158 [2]: ipfs#1159 [3]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29379579 [4]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29498385

Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). I'm not clear on where the 'unix' part of the name comes from (these seem like pretty generic filesystem nodes to me, or if anything, they're POSIX filesystem nodes), but: On Wed, Apr 29, 2015 at 01:30:04PM -0700, Juan Batiz-Benet wrote [3]: > > package fsnode > > i think this package should be called unixfs as that's the > abstraction that this is calling to. So that's what we're going with. [1]: ipfs#1158 [2]: ipfs#1136 (comment) [3]: ipfs#1136 (comment)

Folks should be using the node-returning AddFromReader defined in shell/unixfs instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/unixfs version has a better API and we want to push people in that direction. The shell/unixfs version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2], with the API-release-notes directory proposed here [3] and accepted here [4]. [1]: ipfs#1158 [2]: ipfs#1159 [3]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29379579 [4]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29498385

wking mentioned this issue Apr 28, 2015

Migrating library APIs #1159

Closed

wking mentioned this issue Apr 29, 2015

Replace coreunix.Add with shell/fsnode's AddFromReader #1136

Closed

wking added a commit to wking/go-ipfs that referenced this issue May 2, 2015

core/core: Explain that subpackages are our low-level API

d2a2830

Discussion in [1]. [1]: ipfs#1158

wking added a commit to wking/go-ipfs that referenced this issue May 2, 2015

doc: Add some docs explaining the sub-package layout

6fd877d

Based on [1]. [1]: ipfs#1158

jbenet closed this as completed May 2, 2015

wking mentioned this issue May 4, 2015

remove "testing" imports in files that do not end in _test.go #637

Closed

wking mentioned this issue May 21, 2015

move core mock into its own package #1269

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internal organization for this repository (internal, core, shell, http, cmd, test) #1158

Internal organization for this repository (internal, core, shell, http, cmd, test) #1158

wking commented Apr 28, 2015

jbenet commented Apr 28, 2015

wking commented Apr 29, 2015

wking commented May 2, 2015 via email

jbenet commented May 2, 2015

Internal organization for this repository (internal, core, shell, http, cmd, test) #1158

Internal organization for this repository (internal, core, shell, http, cmd, test) #1158

Comments

wking commented Apr 28, 2015

jbenet commented Apr 28, 2015

wking commented Apr 29, 2015

wking commented May 2, 2015 via email

jbenet commented May 2, 2015