© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
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/
# 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:/
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.
# Knitpicks
I see that you are using redis-server for cache for PyBossa. Do you know we have a redis charm? It would be interesting to see if it could be implemented. The service was, though, running fine.
Because of the errors I have found, I am marking this bug as Incomplete. Please, mark it as New or Fix Committed once you feel you are ready for another review. If you need any help, please make sure to email us at <email address hidden>, ask a question on AskUbuntu with the juju tag, or jump in our IRC channel, #juju on irc.freenode.net. I'm jose there, feel free to ping me at any time.
Again, thanks a lot for your submission of PyBossa to the charm store. I really hope to see this charm in the store soon, as well as seeing the next iteration for the charm!