Code review comment for lp:~dannf/charms/trusty/elasticsearch/unx86ify

Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Tue, Nov 10, 2015 at 8:37 AM dann frazier <email address hidden>
wrote:

> On Mon, Nov 9, 2015 at 12:50 PM, Michael Nelson
> <email address hidden> wrote:
> > Hi Marco, Dann. I thought Dann and I had discussed this earlier, but I
> can't find the record (and old MP?).
>
> I don't recall that - but it has been many months, so I wouldn't be
> surprised. Apologies if I've forgotten - I definitely did not intend
> to dismiss your feedback!
>
> > Generally, I'm not sure that updating the default options with specifics
> like this is a good idea -
> > people with specific cases can override it as needed (and a mention in
> the README might be good).
>
> One of the goals of charms, aiui, is specifically to hide this type of
> complexity. Leaving it up to a user to know that, because they happen
> to be deploying on platform $foo, they need to specify config argument
> $bar seems like magic--. If a charm can be made to *just work*,
> independent of substrate, I'd like to achieve that. That said, I'm not
> married to this specific implementation of making things just work -
> it just seemed to fit the charm design the best, but I'm open to other
> suggestions.
>

Definitely agree - the charm should just work for all supported platforms...

>
> > Specifically in this case, from what I remember, Dann you'd opened a bug
> upstream with elastico about this, and they'd explicitly said they weren't
> testing on this arch:
> >
> > https://github.com/elastic/elasticsearch/issues/9299
> >
> > "Closing this as we don;t want to provide packages for architecture
> which we are not testing. On these architectures Elasticsearch can still be
> installed from the zip or tar.gz distributions"
>
> Which I internalized as "the charm needs to workaround this for the
> foreseeable future". That is, *I* know that this charm is functional
> on arm64 (we demoed this at ODS last spring), and I know that there
> users who *want* to use this charm to evaluate elasticsearch on arm64
> hw. Of course, if there is a standard way to warn charm users they are
> entering into unsupported land, I'd be all for adding such a warning
> here.
>

Yeah - that's what I'm uncertain about - and that's where a note in the
README for running and evaluating on unsupported arches would be easiest.
But I'm only -0, fine as is, just wanted to be explicit that elastico have
marked the above bug as wont-fix because they don't test on that platform.

Thanks Dann.

« Back to merge proposal