[charm-needed] ThinkUp

Bug #891749 reported by Jorge Castro
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Fix Released
Undecided
Nathan Osman

Bug Description

ThinkUp is awesome, we need a charm for it.

The project itself made a "ThinkUp launcher" that asks the user for creds, and then fires up thinkup in EC2. It could probably be used as a basis for a charm:

https://github.com/waxpancake/thinkup-launcher

Tags: new-charm
Revision history for this message
Nathan Osman (george-edison55) wrote :

Here is a branch that contains the code for a ThinkUp charm:

https://code.launchpad.net/~george-edison55/charm/oneiric/thinkup/trunk

Note that ThinkUp needs an SMTP server in order to verify user accounts. I'm not sure whether I should just have the charm install Postfix or if there is another charm that provides SMTP capabilities.

tags: added: new-charm
Marco Ceppi (marcoceppi)
Changed in charm:
assignee: nobody → George Edison (george-edison55)
status: New → In Progress
Revision history for this message
Mark Mims (mark-mims) wrote :

I'd recommend:

first-pass
- install postfix to solve the problem and get the basics of the thinkup charm working

next-pass
- externalize config params like 'install_postfix' 'smtp_server' etc to give the user the option of using an external mail server

next-pass
- it's easy to set postfix up to relay through gmail accounts... add some additional parameters to allow this

thoughts?

Revision history for this message
Nathan Osman (george-edison55) wrote :

Sounds good. I'll add postfix to the ThinkUp charm for now and begin writing a charm for postfix further down the road.

Revision history for this message
Jorge Castro (jorge) wrote :

I get this error trying to relate them:

jorge@lowgirl:~/src$ juju add-relation thinkup mysql
Ambiguous relation 'thinkup mysql'; could refer to:
  'thinkup:db mysql:db' (mysql client / mysql server)
  'thinkup:db mysql:db-admin' (mysql client / mysql server)
2011-11-22 21:04:40,186 ERROR Ambiguous relation 'thinkup mysql'; could refer to:
  'thinkup:db mysql:db' (mysql client / mysql server)
  'thinkup:db mysql:db-admin' (mysql client / mysql server)
jorge@lowgirl:~/src$

Revision history for this message
Mark Mims (mark-mims) wrote : Re: [Bug 891749] Re: [charm-needed] ThinkUp

That's usual... and not an error with the charm.

You have to choose a specific interface if there're two that'll work.
In this case,

     $ juju add-relation thinkup mysql:db

should work.

On 11/22/2011 07:22 PM, Jorge O. Castro wrote:
> I get this error trying to relate them:
>
> jorge@lowgirl:~/src$ juju add-relation thinkup mysql
> Ambiguous relation 'thinkup mysql'; could refer to:
> 'thinkup:db mysql:db' (mysql client / mysql server)
> 'thinkup:db mysql:db-admin' (mysql client / mysql server)
> 2011-11-22 21:04:40,186 ERROR Ambiguous relation 'thinkup mysql'; could refer to:
> 'thinkup:db mysql:db' (mysql client / mysql server)
> 'thinkup:db mysql:db-admin' (mysql client / mysql server)
> jorge@lowgirl:~/src$
>

--
Mark Mims, Ph.D.
Canonical Ltd.
<email address hidden>
+1(512)981-6467

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

Hi George, awesome charm!

Here's my review...

show stoppers:

- please make sure the thinkup.zip payload is cryptographically verified.
One way to do this is to hash the payload, save the hash as part of the charm, then compare after the wget. Another would be to use ssh to download, or to figure out how to tell git to verify certs before pulling from an https url.
Marco has some tools written to help with the first approach: http://bazaar.launchpad.net/~marcoceppi/charm/oneiric/charm-helper/trunk/view/head:/net.sh

recommendations/discussion:

- idempotency. The db-relation-changed hook will be called _several_ times throughout the lifetime of the service. Because of this you gotta make sure it's safe to do so... notice that your hook adds stuff to the database _every_ time it's called. That'll quickly become a problem... It's best practice with charms to put in "guards" or "blockers" to make sure the hook doesn't make those changes more often than you want it to. This isn't a barrier to acceptance, but I'd consider this a bug in the charm.

An example I like to use:

{{{bash

    db_already_configured() {
        # some way to tell that the db has already been configured
    }
    configure_db() {
        # make the big destructive changes... once!
    }
    db_already_configured || configure_db

}}}

- configuration. Do you need to bounce apache after making config changes to thinkup? You might need to bounce apache at the bottom of the config-changed hook. might not though, so check.

Revision history for this message
Nathan Osman (george-edison55) wrote :

Thank you for taking the time to review my charm!

I have now embedded the MD5 hash of the ZIP file in the charm and it is compared to the download file before proceeding with the installation.

I have also added a bit of extra code to the database hook that checks to see if the SQL commands have already been executed or not. This means that the SQL commands will only be executed once.

As for restarting Apache after configuration changes, this is not necessary. The PHP interpreter parses the config. file each time a page is accessed, so there is no need to restart Apache when the file is modified.

Revision history for this message
Nathan Osman (george-edison55) wrote :

On second thought... I just tested the charm one last time and noticed that the plugins weren't being activated properly. I'll look into this tomorrow (it's almost midnight here).

Revision history for this message
Nathan Osman (george-edison55) wrote :

Okay, everything has been tested this time and seems to be working properly. I made some adjustments to the way user accounts are created for the ThinkUp instance. Instead of enabling open registration, I have added three new configuration options that correspond to the administrator account. This allows the administrator to change site-wide settings, such as open registration and plugin configuration.

Incidentally, the requirement for Postfix is now limited to users who want to have more than one account in their ThinkUp instance. There is no requirement for SMTP services if the user only wants one account in their ThinkUp instance.

Please let me know if you have any further questions or concerns regarding this charm.

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

looks good... thanks!

Changed in charm:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.