Comment 4 for bug 822979

Revision history for this message
Mark Mims (mark-mims) wrote :

Hi Luis, thanks for sharing your charm!

Here's my static review to start with:

things to fix:

- charms that are part of the charmstore need to cryptographically verify downloads. Can you please look to see if there're published hashes of the elasticsearch tarball that can be used?

- note that you really don't want the hostname in the rest-relation-changed hook... you can always get the other side's address using `relation-get private-address`.

- what's the rest relation for? Can you add some example usage into the README? Is the intention to eventually proxy requests up to the elasticsearch head?

recommendations / discussion (not necessarily "need to fix"):

- in your install hook, I don't suggest an general "apt-get upgrade"... I think you're better off with updating only the packages you need to update. Was there a particular reason for the upgrade or was it just in the upstream script?

So this is a great idea... I'm excited about where it can go.

I'll leave it "In Progress" for now.

Thanks!