Comment 5 for bug 822979

Hi Mark,

Thanks for your review! I'm excited to try and get the charm in shape for
the charm store. Some answers below.

On Thu, Jun 21, 2012 at 1:00 AM, Mark Mims <email address hidden>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?
>

 I have posted a request for hashes here and there is a first comment
already. Please step in the conversation since I'm not sure exactly what
to recommend here.

https://github.com/elasticsearch/elasticsearch/issues/2050<https://github.com/elasticsearch/elasticsearch/issues/2050#issuecomment-6551195>

> - 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`.
>

I updated the charm locally and am testing. I'll push to my branch when
done.

>
> - 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?
>
>
The rest relation is for querying elasticsearch API:

http://www.elasticsearch.org/guide/reference/api/

This allows querying for search results but also a bunch of administrative
tasks.

I did not define a relation for the elasticsearch transport protocol on
port 9300 simply because I was learning as I went along, but for instance
this is used by logstash when interfacing directly with elastic search.

http://www.elasticsearch.org/guide/reference/modules/transport.html

Looking at that page again it seems there are a bunch of modules (JMX,
memcached, etc...) which would merit defining a relation, so there may be a
lot more work involved in this charm than I set out to implement for my use
case.

>
> 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?
>
>
Ok no prob, this is just a habit that leaked from our current deployment
system which is just a wrapper around aws cloud-formation and we generally
want each new instance to be up to date. I see that it's out of the scope
of juju to guarantee that especially for an individual charm. I'll fix
that.

> So this is a great idea... I'm excited about where it can go.
>
> I'll leave it "In Progress" for now.
>
> Thanks!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/822979
>
> Title:
> Charm needed: Elastic Search
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/822979/+subscriptions
>