Please take a look.


https://codereview.appspot.com/53210044/diff/40001/charm/url.go
File charm/url.go (right):

https://codereview.appspot.com/53210044/diff/40001/charm/url.go#newcode261
charm/url.go:261: func (u *URL) ArchiveName() (string, error) {
On 2014/01/28 08:19:41, fwereade wrote:
> I'm certain this is not part of the charm package -- np with the code,
but it's
> very definitely an internal detail of the state server.

I'll move it to the state package then.

https://codereview.appspot.com/53210044/diff/40001/state/apiserver/charms.go
File state/apiserver/charms.go (right):

https://codereview.appspot.com/53210044/diff/40001/state/apiserver/charms.go#newcode243
state/apiserver/charms.go:243: func GetSHA256(source io.Reader) (string,
int64, error) {
On 2014/01/28 08:19:41, fwereade wrote:
> whereas this feels maybe utilsy -- and we do this *all over the
place*, so
> please hunt them down and make them all use the same utils func,
assuming that
> is practical

Done.

https://codereview.appspot.com/53210044/diff/40001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/53210044/diff/40001/state/apiserver/client/client.go#newcode908
state/apiserver/client/client.go:908: return fmt.Errorf("cannot remove
duplicated charm from storage: %v", err)
On 2014/01/28 08:19:41, fwereade wrote:
> log it, I think, but don't spoil the user's day -- we still managed to
do what
> she asked, after all

Wasn't sure whether to log an error or warning. I'll just log it as an
error then, because it seems serious enough (it shouldn't happen
usually).

https://codereview.appspot.com/53210044/diff/40001/state/apiserver/client/client_test.go
File state/apiserver/client/client_test.go (right):

https://codereview.appspot.com/53210044/diff/40001/state/apiserver/client/client_test.go#newcode1821
state/apiserver/client/client_test.go:1821: close(ops)
On 2014/01/28 08:19:41, fwereade wrote:
> race? we should expect 9 Deletes as well, I think

There is no way to detect storage removals with the dummy provider I
could find. But I am checking below that we do have a single upload at
the end. Not sure what race do you mean?

https://codereview.appspot.com/53210044/diff/40001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/53210044/diff/40001/state/state.go#newcode606
state/state.go:606: err = st.charms.Find(D{{"_id", curl},
{"placeholder", false}}).One(&uploadedCharm)
On 2014/01/28 08:19:41, fwereade wrote:
> placeholder might be false in a new installation, or missing in an old
one

I'll modify the condition, but this applies to other places in the code
as well.

https://codereview.appspot.com/53210044/diff/40001/state/state.go#newcode624
state/state.go:624: Insert: uploadedCharm,
On 2014/01/28 08:19:41, fwereade wrote:
> if there *was* a placeholder charm, the insert would fail, wouldn't
it?

Right, but since I changed how we select the document above it shouldn't
be a problem.

https://codereview.appspot.com/53210044/diff/40001/state/state.go#newcode642
state/state.go:642: StillPlaceholder = D{{"placeholder", true}}
On 2014/01/28 08:19:41, fwereade wrote:
> I think these should probably not be exported

I just reformatted the block - they were there before from Ian's branch
about placeholders. I'll unexport them.

https://codereview.appspot.com/53210044/