Factor out container directory functions.

Factor out container directory functions.

Functions for dealing with the container definition
directories are moved to the container package.

https://code.launchpad.net/~thumper/juju-core/container-directory/+merge/194780

(do not edit description out of merge proposal)

[thumper, 2013-11-12 03:56:35]

Please take a look.

[natefinch, 2013-11-12 21:29:54]

LGTM modulo some pretty trivial things.

https://codereview.appspot.com/24980043/diff/1/container/directory.go
File container/directory.go (right):

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode14
container/directory.go:14: RemovedContainerDir =
"/var/lib/juju/removed-containers"
Can you make these not exported and export them just for tests?  Sorta the whole
point of the encapsulation provided by these methods, right?

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode19
container/directory.go:19: func NewContainerDirectory(containerName string)
(directory string, err error) {
Can we call this NewDirectory?  Seems redundant to have
container.NewContainerDirectory

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode31
container/directory.go:31: func RemoveContainerDirectory(containerName string)
error {
I find the name RemoveContainerDirectory to be confusing, since it's not
actually removing anything, just moving aside.
Also, container.RemoveContainerDirectory(containerName) seems redundant.  Why
not just container.RemoveDirectory(containerName)?

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode51
container/directory.go:51: func jujuContainerDirectory(containerName string)
string {
personally I'd call this something like dirForName or something...
jujuContainerDirectory doesn't really tell me much (also I personally think it's
too long and makes the code harder to read... but I know others think
otherwise).

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode57
container/directory.go:57: func uniqueDirectory(path, name string) (string,
error) {
Can this be put somewhere more generic?  Seems like something we might need to
use quite often (I myself have written this same code at least 3 times in the
past few years).

[thumper, 2013-11-12 21:57:51]

https://codereview.appspot.com/24980043/diff/1/container/directory.go
File container/directory.go (right):

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode14
container/directory.go:14: RemovedContainerDir =
"/var/lib/juju/removed-containers"
On 2013/11/12 21:29:55, nate.finch wrote:
> Can you make these not exported and export them just for tests?  Sorta the
whole
> point of the encapsulation provided by these methods, right?

Unfortunately no, due to the tests being in other packages.

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode19
container/directory.go:19: func NewContainerDirectory(containerName string)
(directory string, err error) {
On 2013/11/12 21:29:55, nate.finch wrote:
> Can we call this NewDirectory?  Seems redundant to have
> container.NewContainerDirectory

Yes we can.

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode31
container/directory.go:31: func RemoveContainerDirectory(containerName string)
error {
On 2013/11/12 21:29:55, nate.finch wrote:
> I find the name RemoveContainerDirectory to be confusing, since it's not
> actually removing anything, just moving aside.
> Also, container.RemoveContainerDirectory(containerName) seems redundant.  Why
> not just container.RemoveDirectory(containerName)?

As far as the caller is concerned, it is removed :-).

Yes, name shortened.

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode51
container/directory.go:51: func jujuContainerDirectory(containerName string)
string {
On 2013/11/12 21:29:55, nate.finch wrote:
> personally I'd call this something like dirForName or something...
> jujuContainerDirectory doesn't really tell me much (also I personally think
it's
> too long and makes the code harder to read... but I know others think
> otherwise).

dirForName should be fine, it is local to the file.

https://codereview.appspot.com/24980043/diff/1/container/directory.go#newcode57
container/directory.go:57: func uniqueDirectory(path, name string) (string,
error) {
On 2013/11/12 21:29:55, nate.finch wrote:
> Can this be put somewhere more generic?  Seems like something we might need to
> use quite often (I myself have written this same code at least 3 times in the
> past few years).

Moved into utils/file.go