Code review comment for lp:~dimitern/juju-core/260-lp-1067979-duplicate-charm

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

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

https://codereview.appspot.com/53210044/diff/1/state/apiserver/client/client.go#newcode885
state/apiserver/client/client.go:885: storageURL, err :=
storage.URL(name)
On 2014/01/27 12:46:16, fwereade wrote:
> I'd prefer to generate unique names than to potentially have multiple
writers to
> the same storage location. I think it'd be best to have multiple
independent
> downloads, and multiple independent uploads, and then just to have a
single
> worker win the race and set a URL that definitely worked in state. No
need to
> rename or anything -- as far as I'm aware, charm URL is totally
independent of
> storage URL anyway, and if it's not that's a bug that should be fixed
anyway ;).

I'll add code to generate a storage URL based on a random UUID string.
If you don't mind, I'll also follow John's suggestion to include the
charm name and revision as prefix, to make the string a bit less
cryptic.

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

https://codereview.appspot.com/53210044/diff/1/state/apiserver/client/client_test.go#newcode1850
state/apiserver/client/client_test.go:1850: }
On 2014/01/27 12:46:16, fwereade wrote:
> in light of the previous comment,we *do* want multiple charms uploaded
(but all
> except one to be deleted). And whichever one isn't deleted should
match the
> archive url of the charm in state.

Will change accordingly.

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

« Back to merge proposal