New charm: PyBossa

[Marvin R.]

new charm for PyBossa

open source framework for making crowdsourcing projects
http://pybossa.com

[Cory Johns (johnsca)]

Marvin,

Thank you for submitting this charm for review. On the whole, it looks great; it deploys without error, the icon seems fine, there are no warnings or errors from `charm proof`, and the README.md is detailed and informative (though, see below). However, I have a few notes and suggestions before I can give my +1 for making this an official Recommended Charm in the store.

First, I think the boilerplate config.yaml should be cleaned up. Until such time as there are actual options to control the deployment of PyBossa, I recommend replacing the config.yaml with:

    options: {}

This way, it is clear at a glance that there are no options to set.

Second, I would prefer the TODO sections in the README were cleaned up, even with simple placeholder text (for example, the config section could simply be: "There are no configuration options supported at this time").

Last, I have a few small suggestions that might help make the hooks a little bit cleaner and easier to follow:

* Static files (e.g., pybossa.ini) can be put in a files/ directory and copied into place, instead of embedded into the install hook (cp $CHARM_DIR/files/pybossa.ini /var/www/pybossa) (also note that $CHARM_DIR will always hold the location of the charm code and files)

* The postgresql charm will create the database you request for you and fire another relation-changed hook once it's ready. It might be cleaner to just check the database name and, if not "pybossa," relation-set it and then exit.

* Given the previous, might it be possible to have the charm use the db relation instead of db-admin?

Again, thank you for this charm submission, and I look forward to getting it approved as a Recommended Charm in the store. Also, just in case you were unaware, as it has not been particularly clear to me until Chuck wrote an excellent blog post*, this charm can be deployed from the store regardless of the state of this review. So don't let my suggested improvements delay you from promoting this very good charm. :)

[1] Chuck's blog post: http://blog.dasroot.net/the-power-of-community-charming/

[José Antonio Rey (jose)]

Hello, Marvin!

First of all, I would like to thank you for the submission of the PyBossa charm to the Charm Store. I believe it would be a great addition to the current charm collection we have! I have performed a full review of the charm and have found the following.

# charm proof

I have ran `charm proof`, our tool to do some basic lint checks, and it has come up with no errors! That's good! Moving on...

# Hooks

I took a look at the config.yaml file. I see that you are using a boilerplate file that contains example configuration options, and I would be for deleting it. When you do and run charm proof, it will only display an I message (informational). A charmer is only unable to promulgate a charm if the charm has W or E, so it would be good.

Also, the config-changed hook restarts the service. This may cause unnecessary service restarts, and deleting it would also generate an I message on charm proof.

Finally, upgrade-charm is also boilerplate and not required, so feel free to delete it! I will get to another hook after I explain the deployment process I've followed.

# Deployment

I followed the instructions found in the README.md file and was able to deploy the service. I have, however, found a couple flaws.

Once the service is deployed and related to postgresql, no ports are opened. I see that there is a line commented out, which mentions "open-port 80". When I read the README, it also mentioned port 80. But on the logs I saw that it was running under port 8080. You should change those references to port 8080 by default. In this case, I see that you are trying to use haproxy so the service can run in port 80, but not everyone wants to deploy haproxy for various reasons, so having port 8080 opened in the machine directly would be awesome to have.

On the other hand, I tried breaking the relation to postgresql and re-adding it. I actually terminated the instance where postgresql was and deployed a new one, re-relating it to pybossa. It looked like pybossa wasn't happy with that, as the hook is not idempotent and the info written in the /var/www/pybossa/.psql file does not change and stays with the information of the first psql server it was related to. In order to fix this, I would recommend to write a pgsql-relation-broken hook, in order to clean all data needed to be cleant after the psql relation has been removed.

# Other issues

I see that you are trying to cd to $charm_dir inside the hooks. This would be a reference to an unbound variable, since the variable needs to be in capital letters. You need to be referencing to $CHARM_DIR instead of $charm_dir, otherwise it cd into the home directory.

Apart from that, all charms that are currently proposed for trusty need to have unit tests included. For that, we have a testing tool, called Amulet. Amulet can help you write Python-based unit tests. You can find more about it here: https://juju.ubuntu.com/docs/tools-amulet.html

Also, you inside the README.md file you are referencing to a local address. It's fine enough to tell users that `juju status` will give you the public address of the instance. And there are also a lot of TODOs listed, you can just remove them and add them later...

[Launchpad Janitor (janitor)]

[Expired for Juju Charms Collection because there has been no activity for 60 days.]