On Fri, Jun 27, 2014 at 4:21 PM, John Meinel <[email protected]> wrote: > Just my quick thought, I think moving it out from "state/api" into just a > top level "api" would be reasonable and a lot less clumsy than trying to > pull it out into an entirely separate repository.
+1 I don't think the api package is useful outside Juju (at this time) and splitting it into another repo just doubles the amount of work. > > I'm not sure if Gustavo realized that "state/apiserver" is the HTTP(-ish, > its really just JSON rpc over a websocket) interface to state. "state/api" > is all go client code (agents or user client code). And it would at least be > really nice for all those things to have an import test that says "does not > depend on anything under "state".) > > It certainly would make it clearer that nothing that is using the api should > import something from, say, apiserver, or state directly. > > John > =:-> > > > On Thu, Jun 26, 2014 at 9:23 PM, roger peppe <[email protected]> wrote: >> >> I have a slightly different proposal, inspired by the recent >> Go proposal for internal imports (http://golang.org/s/go14internal), >> which currently looks like it will actually be implemented. >> >> We move all public facing APIs into top level packages within >> the juju repo, and move everything else under >> github.com/juju/juju/internal >> >> Packages I would suggest should be exported: >> >> github.com/juju/juju/api // moved from state/api >> github.com/juju/juju/cmd/juju >> github.com/juju/juju/cmd/jujud >> ... (all the other commands, but not the cmd package itself) >> github.com/juju/ >> github.com/juju/juju/cmd/internal/jujucmd // containing >> supercommand.go from current cmd >> github.com/juju/juju/juju // but only the API-related pieces >> >> Some packages would want moving, because they're user-facing >> but currently inside packages that we would probably not want to >> export: >> >> github.com/juju/environs/configstore >> >> is the only one I can think of right now. >> >> Others would want splitting. For example, the environs package >> is a mix between user-facing and internal stuff right now. >> It would be great to take out all the user-facing config file stuff >> (that might sit >> well inside the juju package). >> >> Still others would benefit from being made available externally. >> I think the rpc package is probably one of those that would >> sit well inside its own repo. >> >> Some packages could benefit from having their own internal >> directory - the uniter is one of those, for example. The >> apiserver too has many sub-packages that should not really >> be visible to the rest of juju. >> >> In the end, we should end up with an API that it might actually be >> feasible to stabilise. I'd like juju to move under gopkg.in at some >> point, providing useful stability guarantees for external users >> that might want to build Go programs based on our code. >> >> cheers, >> rog. >> >> On 26 June 2014 17:41, Eric Snow <[email protected]> wrote: >> > (I've put a more structured proposal below, but here's some context.) >> > >> > Over the last couple weeks I've been spinning up on the juju code >> > base, which is large enough to dissipate any hope of understanding it >> > all quickly. <wink> Most of what I've focused on is relative to the >> > juju tools and the remote API, both for the sake of backup/restore. >> > >> > One thing that threw me off is that it has not been obvious that the >> > code under `state/api` is the public-facing API for juju's state (as >> > someone recently explained to me). For a while I thought `state/api` >> > held the state API client code, but from what I understand now it >> > actually contains all the public facing code for the juju state API >> > (and [consequently?] for juju as a whole?). In some regard I would >> > expect a public API to be sit in a top-level package (rather than >> > nested down like it is). >> > >> > A couple of other things got in the way. For one, we basically don't >> > have any documentation for the API. I expect that it will mirror the >> > documentation for the juju subcommands/tools pretty closely, but >> > regardless the doc doesn't exist. Setting up godoc for the api >> > package would be great and so would a new page in the juju >> > documentation. I realize there has been some discussion on this point >> > of late, from which I expect a doc will take shape in the short term. >> > In the meantime, we've actually been telling people to wrap calls to >> > the CLI tools rather than using the API. The documentation side of >> > things is a somewhat orthogonal, though topically related, issue. >> > >> > For another, while there has been a pretty good effort to keep the >> > `api` package relatively un-entwined from the rest of juju, there were >> > a few times when I found it hard to follow what was going on. This >> > was particularly true of the underlying state RPC implementation, >> > though at this point things make a lot more sense. Having a separate >> > repo would help delineate the boundary between the API and juju >> > itself, which should in turn help make the API code easier to follow. >> > >> > So... >> > >> > In the interest of understanding juju better and of making the API >> > more accessible, I took a little time to investigate possible >> > improvements. One of the first ones to come to mind was to split >> > `state/api` into its own repo. That smelled like a heavy lift >> > especially considering the many interdependencies between `state/api` >> > and juju proper (though apparently `go` mitigates that somewhat). >> > >> > Undaunted, I gave it an initial stab (see `Implementation` below). >> > The bulk of this effort was fixing all the imports, which I ended up >> > writing a script to solve. All the tests pass on both sides. I >> > wouldn't be comfortable with the split as-is (see `Left to do` below) >> > but it demonstrated to me that it is possible. >> > >> > That said, *possible* should not imply *advisable*, and given my >> > inexperience with the juju project I don't presume to do much more >> > about splitting out an api repo without feedback. At the very least >> > this will be a chance for you to educate me (and presumably others) >> > about the juju code base. :) If the code split is a bad idea then >> > this is your change to officially put the idea to rest. (It would not >> > surprise me if I've misunderstood something important here!) >> > >> > If illumination and a public rejection of the idea are all that come >> > of this I'll still be satisfied. However, I'm hopeful that it's a >> > good enough idea to warrant the effort and that I understand enough >> > about juju at this point to have at least a vague sense of that. :) >> > >> > Thoughts? >> > >> > -eric >> > >> > ++++++++++++++++++++++++++++++++++++++ >> > >> > Proposal >> > ====== >> > * split `github.com/juju/juju/state/api` into its own repo >> > * place the new api repo at `github.com/juju/api` >> > * reduce dependencies in the new repo on the code in the main juju repo >> > * introduce a juju API client interface type >> > * godocs for the `api` package (should happen regardless) >> > * more? (see `Open questions below`) >> > >> > Pros >> > === >> > * helps bless the API as a supported first-class feature of juju >> > * makes the API more discoverable >> > * encourages a stronger separation between the public API (and >> > implementation?) and juju proper, including on the server side >> > * reduces the chance that we inadvertently change/break the public API >> > >> > Cons >> > ==== >> > * touches a lot of code >> > + churn >> > + requires that a lot of (most?) outstanding patches be updated >> > * risk of introducing new bugs >> > * effort to make the split >> > * effort to reduce dependencies in the new repo >> > + the API *implementation* is tightly entwined with juju proper? >> > >> > Open questions >> > =========== >> > Here are some questions that have bearing on this proposal. Some of >> > them would directly impact the scope of the proposal. Others could be >> > addressed separately (to spread the scope out in manageable chunks). >> > >> > * is `state/api` just for the state client/RPC, juju state in general, >> > or juju as a whole? >> > From what I understand, it is the middle one (though I originally >> > thought it was the first). >> > * is there other public juju API other than the state-related API? >> > * should the `api` repo contains *all* public-facing juju API >> > (state-related or not), regardless of whether or not `state/API` >> > currently does? >> > This would imply that the `api` package would be restructured to >> > reflect various APIs (e.g. `api/state/client`). >> > * should `api` hold just interfaces and constants, leaving the actual >> > implementation in the main juju repo (or just tucked away within the >> > new `api` package)? >> > It could also include a public wrapper around the implementation. >> > * would it be worth restructuring the `api` package to group the >> > different methods/constants/types by component (e.g. charms/state >> > archive/tools/services/units/etc.)? >> > This would particularly impact `api/params`. >> > * how much should dependencies in juju proper on the `api` package be >> > dialed back? >> > I'd think it would be as much as possible. More encapsulation >> > around the API/RPC part of juju would be good. <waves hands> This >> > could go as far as to include portions (most?) of the server side of >> > the state RPC within `api`. >> > * should the underlying state client RPC implementation move over with >> > `state/api` (or even into its own repo)? >> > Like the client implementation, the RPC code could be tucked away >> > within `api`. >> > >> > Implementation >> > =========== >> > I've put up a rough implementation of the split that passes the test >> > suite on both sides: >> > >> > https://github.com/ericsnowcurrently/juju-api >> > https://github.com/ericsnowcurrently/juju/tree/state-api-repo >> > >> > Keep in mind that it's already out of sync, but the point is that it >> > works. :) I also have a script that updates the imports (mitigating >> > the burden of doing so manually). >> > >> > Left to do >> > ======= >> > * reduce dependencies in the new api repo >> > * add an API client interface type >> > >> > Alternatives >> > ========= >> > * apply some of the ideas here to `state/api` rather than in a new repo >> > * move some or all of `state/api` into a new top-level `api` package >> > in juju proper >> > >> > -- >> > Juju-dev mailing list >> > [email protected] >> > Modify settings or unsubscribe at: >> > https://lists.ubuntu.com/mailman/listinfo/juju-dev >> >> -- >> Juju-dev mailing list >> [email protected] >> Modify settings or unsubscribe at: >> https://lists.ubuntu.com/mailman/listinfo/juju-dev > > > > -- > Juju-dev mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > -- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
