Re: Splitting out state/api into its own repo
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. 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 rogpe...@gmail.com 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 eric.s...@canonical.com 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
Fwd: Splitting out state/api into its own repo
[Hmm, I'm not sure this got through originally, as I sent it from my non-work gmail account] 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 eric.s...@canonical.com 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
Re: Splitting out state/api into its own repo
On Fri, Jun 27, 2014 at 4:21 PM, John Meinel j...@arbash-meinel.com 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 rogpe...@gmail.com 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 eric.s...@canonical.com 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...
Re: Splitting out state/api into its own repo
The API provides the business logic to Go and non-Go clients and uses the state. This dependency could best be demonstrated, also for new maintainers and contributors, by having the API as a top level below github.com/juju/juju. Here it also best should provide the two packages client and sever. Somhow the naming api and apiserver below state seem inconsequent. The API is an essential part of Juju core, so +1 for NOT creating an own repository. mue On Fri, Jun 27, 2014 at 8:51 AM, David Cheney david.che...@canonical.com wrote: On Fri, Jun 27, 2014 at 4:21 PM, John Meinel j...@arbash-meinel.com 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 rogpe...@gmail.com 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 eric.s...@canonical.com 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
Fwd: Splitting out state/api into its own repo
On 27 June 2014 07:51, David Cheney david.che...@canonical.com wrote: On Fri, Jun 27, 2014 at 4:21 PM, John Meinel j...@arbash-meinel.com 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. Do you mean that the API package isn't useful *from* outside Juju, or that the API package isn't useful *independently of* Juju? If the latter, I totally agree (the whole point is that it integrates with Juju) but if the former, I disagree. If we are to allow any external Go programs that use Juju (and I think we should - we should act as good citizens in the Go ecosystem) then the API package is the only way to do it. We shouldn't force people to write their own API interface just because we're not prepared to support our own. BTW, I think it would be a good idea to split off the agent parts of the API from the client parts - the former should not be considered public. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Actions document - suggested changes
On Thu, Jun 26, 2014 at 6:47 AM, Tim Penhey tim.pen...@canonical.com
wrote:
On reading the spec[1] and looking at the state documents as they are in
trunk now, I was quickly coming to the conclusion that the documents
need to change.
I agree with that, but...
Right now we have two action related documents:
...
I think these should be combined,
Strong -1. We've made this mistake too many times in the past; we need to
group the fields in separate docs according to who needs to read/write
them, so we don't end up with embarrassments like the N^2 add-unit problem.
and there are quite a few missing
fields that are clearly expected from the spec:
invoked: TIME
started: TIME
finished: TIME
files:
- this is a tricky one
Obviously we are also missing the user that asked for the action.
Agreed. These fields will come when we're ready to use them.
Consider the following structure:
type actionDoc struct {
...
// UnitName is the name of the unit that the action will
// run on
UnitName string
Not necessarily a unit. We need service actions, and probably one day stack
actions (even if they're likely to be represented as service actions).
...
// User is the user that requested this action
User string
Not necessarily a user. I think there's a class of service-level action
that will only be possible if the leader has a way of invoking actions on
its followers.
...
// ReturnCode is the result of the action
ReturnCode *int
Not so convinced that this is really what we want, more on this later.
// Results are defined and set by the action themselves.
Results map[string]interface{}
// Files is interesting, in that they should probably live
// in the environment file storage, what was cloud storage
// and is now GridFS, under some particular directory:
// perhaps /actions/uuid/filename
// Both stdout and stderr are recorded as files. They are
// only added if they are not empty.
// Other files can be added by the action.
Files []string
I'm concerned that this overlaps unhelpfully with other features. Hook
output is already logged elsewhere, and I think I'd prefer to just make
sure they're badged properly so that they're easily accessible via
debug-log.
Other files, and/or the possibility of multiple output documents, feel like
they're an independent feature. Sorry I didn't flag this before, but that
spec has had some interesting changes since I +1ed it -- in short, I'm
concerned that we're sacrificing orthogonality for the sake of gilding the
actions feature. I think a single output map for the action results will
give us a lot of value, and I anticipate that a separate output feature --
in which we can store structured maps, or binary blobs -- is valuable in
many contexts, not just in actions.
status becomes an emergent property:
if Started is nil
pending
if Finished is nil
started
if ReturnCode is 0 (not nil)
success
else
failure
Up to a point, yes, but I think there's a bit more granularity to status.
There's also the possibility that the unit agent went down mid-action; and
I wouldn't be surprised if we came up with other edge cases. Basically I
think we really do want Status instead of ReturnCode; while we could
plausibly infer pending/started statuses from other fields, but I'm not
sure it's *that* much of a win -- we may as well set started when we set
the started time, and then we can just get the status by looking without
having to infer anything. (Although we would have to infer pending from the
lack of an ActionResult anyway.)
Queues, Lists, and Results are just queries across this document set for
the environment, optionally scoped to unit names and users.
Not sure how to watch them. Remember that we can't really do db queries
inside watches, because any blocked watcher blocks the whole watching
infrastructure. This is not to say that we *can't* get around it, but it's
a bunch of complexity for relatively little benefit IMO; and people
generally find the watchers hard enough to deal with without adding
additional layers.
Does that seem reasonable to other people?
Some of it does, yeah. My overriding concern is about just sticking
everything in the same doc just because they're conceptually related: I
really don't think it's a good idea.
Cheers
William
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Port ranges - restricting opening and closing ranges
+1 on an opened-ports hook tool, I've added it to the task list On Fri, Jun 27, 2014 at 9:41 AM, William Reade william.re...@canonical.com wrote: Agreed. Note, though, that we'll want to give charms a way to know what ports they have already opened: I think this is a case where look-before-you-leap maybe beats easier-ask-forgiveness-than-permission (and the consequent requirement that error messages be parsed...). An opened-ports hook tool should do the trick. On Thu, Jun 26, 2014 at 9:18 PM, Gustavo Niemeyer gust...@niemeyer.net wrote: +1 to Mark's point. Handling exact matches is much easier, and does not prevent a fancier feature later, if there's ever the need. On Thu, Jun 26, 2014 at 3:38 PM, Mark Ramm-Christensen (Canonical.com) mark.ramm-christen...@canonical.com wrote: My belief is that as long as the error messages are clear, and it is easy to close 8000-9000 and then open 8000-8499 and 8600-9000, we are fine. Of course it is nicer if we can do that automatically for you, but I don't see why we can't add that later, and I think there is a value in keeping a port-range as an atomic data-object either way. --Mark Ramm On Thu, Jun 26, 2014 at 2:11 PM, Domas Monkus domas.mon...@canonical.com wrote: Hi, me and Matthew Williams are working on support for port ranges in juju. There is one question that the networking model document does not answer explicitly and the simplicity (or complexity) of the implementation depends greatly on that. Should we only allow units to close exactly the same port ranges that they have opened? That is, if a unit opens the port range [8000-9000], can it later close ports [8500-8600], effectively splitting the previously opened port range in half? Domas -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- gustavo @ http://niemeyer.net -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Splitting out state/api into its own repo
I think these are the results we are looking to get out of this exercise: 1. Have a well-defined and well-documented API that third parties may consume. 2. Have a clean and well-documented Go implementation of an API wrapper which third parties may reuse. 3. Make both of these easy to find. #1 is independent of the location of the code for the API server. No one needs to see the implementation of the server in order to be able to consume the API. They need good documentation and the IP address of the state server. If they need to care about implementation, we're doing something drastically wrong. #2 should absolutely be documented using godoc and live with the code, and have the code somewhere easily accessible in some repo, preferably without needing to download the entire juju codebase. Ideally, #1 would live at http://juju.ubuntu.com/docs/v1.18.4/API.html FWIW, I really like Roger's idea of automatically generating the API documentation from the code. On Fri, Jun 27, 2014 at 5:31 AM, roger peppe roger.pe...@canonical.com wrote: On 27 June 2014 10:05, William Reade william.re...@canonical.com wrote: I think one of the biggest problems is the naming: state/api is a hackish and minimal api client implementation, while state/apiserver is where the actual api is defined... except the params package, which for some reason lives under state/api. I think the most important actions are: * move state/api/params under state/apiserver * move state/apiserver to the top level, and make sure it's clearly documented Why would we want to do that? No non-juju package should ever be running the API server or calling any of its code directly, should it? Someone connecting directly to the API and sending JSON should not need to look at godoc package documentation to infer what their messages should look like. I agree that the API should be fully documented, and that the current api package does not expose some capabilities of the underlying API implementation, notably the ability to make bulk calls (although in practice there's no advantage to doing that currently). However that's an argument for a) fully documenting the API and b) updating the API client side to more closely reflect the actual API. I have long thought that it should not be too hard to generate API documentation automatically from the server-side code. This was part of the original motivation for the rpcreflect package. I have code that generates JSON description of all the calls in the API - if integrated with comments from the server-side implementation methods, I think this could be pretty good, and avoid the need to maintain a separate API doc. Likewise the client side code can be automatically generated (I have done that before too), which would make for a Go API interface which exactly reflected the server side. As John says, api/params is an important part of the client-side interface - it defines the data structures that the server and client side have in common. cheers, rog. Then I'd be keen to separate the internal api client code from the external one; and at that point I'd be happy to move the external api client code into its own repo. There's no disadvantage to having that code external, because we can't afford to break our external api clients regardless; for the internal ones we have more power and control, because we're the only ones who have to deal with the impact of change. (This would then involve separating the protocol-level code out somewhere *else*, so that we could reuse it both internally and externally; and we'd probably want both the server and client protocol parts together; but I think that the point where we can reasonably move a package outside the main repo is some way away regardless, so I'm not keen to focus on it at the moment.) Cheers William On Fri, Jun 27, 2014 at 10:01 AM, roger peppe roger.pe...@canonical.com wrote: On 27 June 2014 07:51, David Cheney david.che...@canonical.com wrote: On Fri, Jun 27, 2014 at 4:21 PM, John Meinel j...@arbash-meinel.com 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. Do you mean that the API package isn't useful *from* outside Juju, or that the API package isn't useful *independently of* Juju? If the latter, I totally agree (the whole point is that it integrates with Juju) but if the former, I disagree. If we are to allow any external Go programs that use Juju (and I think we should - we should act as good citizens in the Go ecosystem) then the API package is the only way to do it. We
Re: Splitting out state/api into its own repo
On Fri, Jun 27, 2014 at 3:05 AM, William Reade william.re...@canonical.com wrote: I think one of the biggest problems is the naming: state/api is a hackish and minimal api client implementation, while state/apiserver is where the actual api is defined... except the params package, which for some reason lives under state/api. I think the most important actions are: * move state/api/params under state/apiserver * move state/apiserver to the top level, and make sure it's clearly documented Then I'd be keen to separate the internal api client code from the external one; and at that point I'd be happy to move the external api client code into its own repo. There's no disadvantage to having that code external, because we can't afford to break our external api clients regardless; for the internal ones we have more power and control, because we're the only ones who have to deal with the impact of change. (This would then involve separating the protocol-level code out somewhere *else*, so that we could reuse it both internally and externally; and we'd probably want both the server and client protocol parts together; but I think that the point where we can reasonably move a package outside the main repo is some way away regardless, so I'm not keen to focus on it at the moment.) This all sounds great. It's basically what I was trying to suggest. :) I just don't have as clear a picture of the distinction between internal/external client, which is where I went awry. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Splitting out state/api into its own repo
On Fri, Jun 27, 2014 at 5:36 AM, Nate Finch nate.fi...@canonical.com wrote: have the code somewhere easily accessible in some repo, preferably without needing to download the entire juju codebase. Good point. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Splitting out state/api into its own repo
On Thu, Jun 26, 2014 at 11:04 AM, Gustavo Niemeyer gust...@niemeyer.net wrote: Hey Eric, Some comments below, offering a slightly different perspective to be used as a data point in your quest. That was totally helpful. And with the further discussion (thanks everyone!), I'm still hopeful that something good will come of this (besides the benefit to my understanding). :) In the end I'd argue that anything we can do (within reason) to make the code easier to understand is a win. That benefits not only newcomers to the code like myself, but the project as a whole; if done thoughtfully the code often ends up with better boundaries between components and between internal-/external-facing roles. Regarding understandability, I can't speak to what effort has gone into a complex system like juju (though I suspect it has been significant), but my experience thus far has been that the code (specifically the API code) in its current state hasn't been the easiest to follow. I expect the complexity of the system has a lot to do with that. From the discussion so far it sounds like there are things we can do about it, which is great! For 3, splitting it off not only seems to needlessly increase the maintenance burden, but also feels incorrect from the orthogonality standpoint: state/api maps state into a public API, and is a critical dependency for juju to work at all. Yeah, as William pointed out, splitting the API client out into its own repo would only make sense after distilling it down to the actual public-facing part. Anyway, thanks again for the insight! Everyone's been really helpful as I've been spinning up. -eric -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Splitting out state/api into its own repo
Thanks to all the responses thus far. While the discussion has been insightful, I'm also hopeful that it will still be fruitful. :) Understandably the responses have been focused on the main point of my original post, splitting out the API client into its own repo. However, there were a few other points/questions that I think would be worth revisiting (or are effectively still on the table) that were otherwise tucked away here. Following is the original email with the everything but those points snipped away (and edits in square brackets). -eric 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). * introduce a juju API client interface type * 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` [package] contains *all* public-facing juju API (state-related or not), regardless of whether or not `state/API` currently does? * 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. * should the underlying state client RPC implementation move over with `state/api` (or even into its own repo)? 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 -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Thoughts to keep in mind for Code Review
I also found this useful. This is specific to Go and more about the details of the code itself than the abstract of the review as a whole. https://code.google.com/p/go-wiki/wiki/CodeReviewComments On Wed, Jun 25, 2014 at 9:31 AM, Gustavo Niemeyer gust...@niemeyer.net wrote: Thanks, John. Several nice ideas there. I especially like the data backing the first few points.. it provides evidence to something we intuitively understand. I also wrote some points about this same topic, but from a slightly different perspective, last year: http://blog.labix.org/2013/02/06/ethics-for-code-reviewers On Wed, Jun 25, 2014 at 1:20 AM, John Meinel j...@arbash-meinel.com wrote: An interesting article from IBM: http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/ There is a pretty strong bias for we found these results and look at how our tool makes it easier to follow these guidelines, but the core results are actually pretty good. I certainly recommend reading it and keeping some of it in mind while you're both coding and reviewing. (Particularly how long should code review take, and how much code should be put up for review at a time.) Another trick that we haven't made much use of is to annotate the code we put up for review. We have the summary description, but you can certainly put some inline comments on your own proposal if you want to highlight areas more clearly. John =:- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- gustavo @ http://niemeyer.net -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Wayne Witzel III wayne.wit...@canonical.com -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
